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 3/5] KEYS: encrypted: sanitize all key material Date: Fri, 21 Apr 2017 01:30:35 -0700 [thread overview] Message-ID: <20170421083037.12746-4-ebiggers3@gmail.com> (raw) In-Reply-To: <20170421083037.12746-1-ebiggers3@gmail.com> 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> --- 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 0010955d7876..1ca895e7e56a 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -397,7 +397,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(derived_key, derived_buf, derived_buf_len); - kfree(derived_buf); + kzfree(derived_buf); return ret; } @@ -533,6 +533,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; } @@ -571,6 +572,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; } @@ -722,6 +724,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; } @@ -828,13 +831,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; } @@ -843,8 +846,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); } /* @@ -902,7 +904,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; } @@ -960,33 +962,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 = { -- 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 3/5] KEYS: encrypted: sanitize all key material Date: Fri, 21 Apr 2017 01:30:35 -0700 [thread overview] Message-ID: <20170421083037.12746-4-ebiggers3@gmail.com> (raw) In-Reply-To: <20170421083037.12746-1-ebiggers3@gmail.com> 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> --- 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 0010955d7876..1ca895e7e56a 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -397,7 +397,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(derived_key, derived_buf, derived_buf_len); - kfree(derived_buf); + kzfree(derived_buf); return ret; } @@ -533,6 +533,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; } @@ -571,6 +572,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; } @@ -722,6 +724,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; } @@ -828,13 +831,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; } @@ -843,8 +846,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); } /* @@ -902,7 +904,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; } @@ -960,33 +962,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 = { -- 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:32 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 ` Eric Biggers [this message] 2017-04-21 8:30 ` [PATCH 3/5] KEYS: encrypted: sanitize all key material Eric Biggers 2017-04-21 8:30 ` [PATCH 4/5] KEYS: trusted: " Eric Biggers 2017-04-21 8:30 ` 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-4-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.