linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: jmorris@namei.org
Cc: Eric Biggers <ebiggers@google.com>,
	David Safford <safford@us.ibm.com>,
	linux-kernel@vger.kernel.org, dhowells@redhat.com,
	linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
	Mimi Zohar <zohar@linux.vnet.ibm.com>
Subject: [PATCH 15/23] KEYS: encrypted: sanitize all key material
Date: Thu, 08 Jun 2017 14:49:11 +0100	[thread overview]
Message-ID: <149692975135.11452.9791971822965563991.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <149692963884.11452.7673998701432248814.stgit@warthog.procyon.org.uk>

From: Eric Biggers <ebiggers@google.com>

For keys of type "encrypted", consistently zero sensitive key material
before freeing it.  This was already being done for the decrypted
payloads of encrypted keys, but not for the master key and the keys
derived from the master key.

Out of an abundance of caution and because it is trivial to do so, also
zero buffers containing the key payload in encrypted form, although
depending on how the encrypted-keys feature is used such information
does not necessarily need to be kept secret.

Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Safford <safford@us.ibm.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 security/keys/encrypted-keys/encrypted.c |   31 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 5c98c2fe03f0..bb6324d1ccec 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -375,7 +375,7 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
 	memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
 	       master_keylen);
 	ret = calc_hash(hash_tfm, derived_key, derived_buf, derived_buf_len);
-	kfree(derived_buf);
+	kzfree(derived_buf);
 	return ret;
 }
 
@@ -507,6 +507,7 @@ static int datablob_hmac_append(struct encrypted_key_payload *epayload,
 	if (!ret)
 		dump_hmac(NULL, digest, HASH_SIZE);
 out:
+	memzero_explicit(derived_key, sizeof(derived_key));
 	return ret;
 }
 
@@ -545,6 +546,7 @@ static int datablob_hmac_verify(struct encrypted_key_payload *epayload,
 		dump_hmac("calc", digest, HASH_SIZE);
 	}
 out:
+	memzero_explicit(derived_key, sizeof(derived_key));
 	return ret;
 }
 
@@ -701,6 +703,7 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload,
 out:
 	up_read(&mkey->sem);
 	key_put(mkey);
+	memzero_explicit(derived_key, sizeof(derived_key));
 	return ret;
 }
 
@@ -807,13 +810,13 @@ static int encrypted_instantiate(struct key *key,
 	ret = encrypted_init(epayload, key->description, format, master_desc,
 			     decrypted_datalen, hex_encoded_iv);
 	if (ret < 0) {
-		kfree(epayload);
+		kzfree(epayload);
 		goto out;
 	}
 
 	rcu_assign_keypointer(key, epayload);
 out:
-	kfree(datablob);
+	kzfree(datablob);
 	return ret;
 }
 
@@ -822,8 +825,7 @@ static void encrypted_rcu_free(struct rcu_head *rcu)
 	struct encrypted_key_payload *epayload;
 
 	epayload = container_of(rcu, struct encrypted_key_payload, rcu);
-	memset(epayload->decrypted_data, 0, epayload->decrypted_datalen);
-	kfree(epayload);
+	kzfree(epayload);
 }
 
 /*
@@ -881,7 +883,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
 	rcu_assign_keypointer(key, new_epayload);
 	call_rcu(&epayload->rcu, encrypted_rcu_free);
 out:
-	kfree(buf);
+	kzfree(buf);
 	return ret;
 }
 
@@ -939,33 +941,26 @@ static long encrypted_read(const struct key *key, char __user *buffer,
 
 	up_read(&mkey->sem);
 	key_put(mkey);
+	memzero_explicit(derived_key, sizeof(derived_key));
 
 	if (copy_to_user(buffer, ascii_buf, asciiblob_len) != 0)
 		ret = -EFAULT;
-	kfree(ascii_buf);
+	kzfree(ascii_buf);
 
 	return asciiblob_len;
 out:
 	up_read(&mkey->sem);
 	key_put(mkey);
+	memzero_explicit(derived_key, sizeof(derived_key));
 	return ret;
 }
 
 /*
- * encrypted_destroy - before freeing the key, clear the decrypted data
- *
- * Before freeing the key, clear the memory containing the decrypted
- * key data.
+ * encrypted_destroy - clear and free the key's payload
  */
 static void encrypted_destroy(struct key *key)
 {
-	struct encrypted_key_payload *epayload = key->payload.data[0];
-
-	if (!epayload)
-		return;
-
-	memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
-	kfree(key->payload.data[0]);
+	kzfree(key->payload.data[0]);
 }
 
 struct key_type key_type_encrypted = {

  parent reply	other threads:[~2017-06-08 13:49 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-08 13:47 [PATCH 00/23] KEYS: Fixes David Howells
2017-06-08 13:47 ` [PATCH 01/23] security/keys: add CONFIG_KEYS_COMPAT to Kconfig David Howells
2017-06-08 13:47 ` [PATCH 02/23] security: use READ_ONCE instead of deprecated ACCESS_ONCE David Howells
2017-06-08 13:47 ` [PATCH 03/23] KEYS: fix refcount_inc() on zero David Howells
2017-06-08 13:47 ` [PATCH 04/23] X.509: Fix error code in x509_cert_parse() David Howells
2017-06-08 13:47 ` [PATCH 05/23] KEYS: Delete an error message for a failed memory allocation in get_derived_key() David Howells
2017-06-08 13:48 ` [PATCH 06/23] KEYS: put keyring if install_session_keyring_to_cred() fails David Howells
2017-06-08 13:48 ` [PATCH 07/23] KEYS: encrypted: avoid encrypting/decrypting stack buffers David Howells
2017-06-08 13:48 ` [PATCH 08/23] KEYS: encrypted: fix buffer overread in valid_master_desc() David Howells
2017-06-08 13:48 ` [PATCH 09/23] KEYS: encrypted: fix race causing incorrect HMAC calculations David Howells
2017-06-08 13:48 ` [PATCH 10/23] KEYS: encrypted: use constant-time HMAC comparison David Howells
2017-06-08 13:48 ` [PATCH 11/23] KEYS: fix dereferencing NULL payload with nonzero length David Howells
2017-06-08 13:48 ` [PATCH 12/23] KEYS: fix freeing uninitialized memory in key_update() David Howells
2017-06-08 13:48 ` [PATCH 13/23] KEYS: sanitize add_key() and keyctl() key payloads David Howells
2017-06-08 13:49 ` [PATCH 14/23] KEYS: user_defined: sanitize " David Howells
2017-06-08 13:49 ` David Howells [this message]
2017-06-08 13:49 ` [PATCH 16/23] KEYS: trusted: sanitize all key material David Howells
2017-06-08 13:49 ` [PATCH 17/23] KEYS: sanitize key structs before freeing David Howells
2017-06-08 13:49 ` [PATCH 18/23] KEYS: DH: forbid using digest_null as the KDF hash David Howells
2017-06-08 13:49 ` [PATCH 19/23] KEYS: DH: don't feed uninitialized "otherinfo" into KDF David Howells
2017-06-08 13:49 ` [PATCH 20/23] KEYS: DH: ensure the KDF counter is properly aligned David Howells
2017-06-08 13:49 ` [PATCH 21/23] KEYS: DH: add __user annotations to keyctl_kdf_params David Howells
2017-06-08 13:50 ` [PATCH 22/23] crypto : asymmetric_keys : verify_pefile:zero memory content before freeing David Howells
2017-06-08 13:50 ` [PATCH 23/23] KEYS: Convert KEYCTL_DH_COMPUTE to use the crypto KPP API David Howells
2017-06-08 14:33 ` [PATCH 00/23] KEYS: Fixes James Morris
2017-06-08 14:49 ` David Howells

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=149692975135.11452.9791971822965563991.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=ebiggers@google.com \
    --cc=jmorris@namei.org \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).