Re: crypto: zeroization of sensitive data in af_alg
On 11/11/2014 05:16 AM, Stephan Mueller wrote: ... That is a good idea. Herbert: I can prepare a patch that uses memzero_explicit. However, your current tree does not yet implement that function as it was added to Linus' tree after you pulled from it. Yep, Ted took it [1] on top of the random driver fix as discussed. [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7185ad2672a7d50bc384de0e38d90b75d99f3d82 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: crypto: zeroization of sensitive data in af_alg
On Sun, Nov 09, 2014 at 11:33:52PM +0100, Stephan Mueller wrote: while working on the AF_ALG interface, I saw no active zeroizations of memory that may hold sensitive data that is maintained outside the kernel crypto API cipher handles. I think the following memory segments fall under that category: Are you talking about temporary data that we generate as part of the processing? If so they should be zeroed by the entity that generates them. However, I am failing to find the right spot to add a zeroization for the latter one, i.e. the code that handles the pages send in by the user or the pages that are returned by the crypto API. Initially I thought skcipher_pull_sgl is a good spot for the symmetric ciphers as it evicts the used pages out of the scope of the kernel crypto API. I added a clear_page(sg_page(sg+1)) as well as memset(sg_page(sg+1), 0, plen) right before the put_page call. All that I got in return was a BUG() from the memory management layer. I don't think I understand what exactly you're trying to zero. Can you give an example? Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: crypto: zeroization of sensitive data in af_alg
Am Montag, 10. November 2014, 22:05:18 schrieb Herbert Xu: Hi Herbert, On Sun, Nov 09, 2014 at 11:33:52PM +0100, Stephan Mueller wrote: while working on the AF_ALG interface, I saw no active zeroizations of memory that may hold sensitive data that is maintained outside the kernel crypto API cipher handles. I think the following memory segments fall under that category: Are you talking about temporary data that we generate as part of the processing? If so they should be zeroed by the entity that generates them. I currently see that the IV buffer (owned by skcipher) and the message digest buffer (owned by hash) are not memset(0) before freeing them. I agree that both are not really sensitive data. But wouldn't it be prudent to memset(0) them nonetheless in the skcipher_sock_destruct and hash_sock_destruct functions, respectively? However, I am failing to find the right spot to add a zeroization for the latter one, i.e. the code that handles the pages send in by the user or the pages that are returned by the crypto API. Initially I thought skcipher_pull_sgl is a good spot for the symmetric ciphers as it evicts the used pages out of the scope of the kernel crypto API. I added a clear_page(sg_page(sg+1)) as well as memset(sg_page(sg+1), 0, plen) right before the put_page call. All that I got in return was a BUG() from the memory management layer. I don't think I understand what exactly you're trying to zero. Can you give an example? Apologies, my bad as I did not check get_user_pages_fast well enough. I see now that we operate on the pages in user space directly without copy_from_user that would imply a kernel-internal copy. Please disregard my comment. Thanks, -- Ciao Stephan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: crypto: zeroization of sensitive data in af_alg
On Tue, Nov 11, 2014 at 03:06:32AM +0100, Stephan Mueller wrote: Am Montag, 10. November 2014, 22:05:18 schrieb Herbert Xu: Hi Herbert, On Sun, Nov 09, 2014 at 11:33:52PM +0100, Stephan Mueller wrote: while working on the AF_ALG interface, I saw no active zeroizations of memory that may hold sensitive data that is maintained outside the kernel crypto API cipher handles. I think the following memory segments fall under that category: Are you talking about temporary data that we generate as part of the processing? If so they should be zeroed by the entity that generates them. I currently see that the IV buffer (owned by skcipher) and the message digest buffer (owned by hash) are not memset(0) before freeing them. I agree that both are not really sensitive data. But wouldn't it be prudent to memset(0) them nonetheless in the skcipher_sock_destruct and hash_sock_destruct functions, respectively? Yes please submit your patches. Apologies, my bad as I did not check get_user_pages_fast well enough. I see now that we operate on the pages in user space directly without copy_from_user that would imply a kernel-internal copy. Please disregard my comment. OK. Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: crypto: zeroization of sensitive data in af_alg
On Sun, Nov 9, 2014 at 5:33 PM, Stephan Mueller smuel...@chronox.de wrote: while working on the AF_ALG interface, I saw no active zeroizations of memory that may hold sensitive data that is maintained outside the kernel crypto API cipher handles. ... I think I found the location for the first one: hash_sock_destruct that should be enhanced with a memset(0) of ctx-result. See also a thread titled memset() in crypto code? on the linux crypto list. The claim is that gcc can optimise memset() away so you need a different function to guarantee the intended results. There's a patch to the random driver that uses a new function memzero_explicit(), and one of the newer C standards has a different function name for the purpose. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: crypto: zeroization of sensitive data in af_alg
Am Montag, 10. November 2014, 21:55:43 schrieb Sandy Harris: Hi Sandy, Herbert, On Sun, Nov 9, 2014 at 5:33 PM, Stephan Mueller smuel...@chronox.de wrote: while working on the AF_ALG interface, I saw no active zeroizations of memory that may hold sensitive data that is maintained outside the kernel crypto API cipher handles. ... I think I found the location for the first one: hash_sock_destruct that should be enhanced with a memset(0) of ctx-result. See also a thread titled memset() in crypto code? on the linux crypto list. The claim is that gcc can optimise memset() away so you need a different function to guarantee the intended results. There's a patch to the random driver that uses a new function memzero_explicit(), and one of the newer C standards has a different function name for the purpose. That is a good idea. Herbert: I can prepare a patch that uses memzero_explicit. However, your current tree does not yet implement that function as it was added to Linus' tree after you pulled from it. Shall I now still use memset(0) or prepare a patch that does not yet compile by using memzero_explicit? -- Ciao Stephan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: crypto: zeroization of sensitive data in af_alg
On Tue, Nov 11, 2014 at 05:16:54AM +0100, Stephan Mueller wrote: Shall I now still use memset(0) or prepare a patch that does not yet compile by using memzero_explicit? Just send the patch with the memzer_explicit and I'll make sure that I pull the requisite changes in before I apply your patch. Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
crypto: zeroization of sensitive data in af_alg
Hi Herbert, while working on the AF_ALG interface, I saw no active zeroizations of memory that may hold sensitive data that is maintained outside the kernel crypto API cipher handles. I think the following memory segments fall under that category: * message digest * IV * plaintext / ciphertext handed in by consumer * ciphertext / plaintext that is send back to the consumer May I ask whether such zeroizations are present? At least I did not find it. If we conclude that there is a need for adding such zeroizations, I checked the code for the appropriate locations: I think I found the location for the first one: hash_sock_destruct that should be enhanced with a memset(0) of ctx-result. I have a patch ready which is tested and works. For the IV, I think I found the spot as well: skcipher_sock_destruct. This function should be enhanced with a memset(0) of ctx-iv. Again, I have a patch ready which is tested and works. However, I am failing to find the right spot to add a zeroization for the latter one, i.e. the code that handles the pages send in by the user or the pages that are returned by the crypto API. Initially I thought skcipher_pull_sgl is a good spot for the symmetric ciphers as it evicts the used pages out of the scope of the kernel crypto API. I added a clear_page(sg_page(sg+1)) as well as memset(sg_page(sg+1), 0, plen) right before the put_page call. All that I got in return was a BUG() from the memory management layer. Then I tried the same in af_alg_free_sg() as this function is used by algif_hash.c -- with the same result. That makes me wonder: where should such a zeroization call be added? Thanks -- Ciao Stephan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
