* crypto: zeroization of sensitive data in af_alg
@ 2014-11-09 22:33 Stephan Mueller
2014-11-10 14:05 ` Herbert Xu
2014-11-11 2:55 ` Sandy Harris
0 siblings, 2 replies; 8+ messages in thread
From: Stephan Mueller @ 2014-11-09 22:33 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-kernel, linux-crypto
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-09 22:33 crypto: zeroization of sensitive data in af_alg Stephan Mueller
@ 2014-11-10 14:05 ` Herbert Xu
2014-11-11 2:06 ` Stephan Mueller
2014-11-11 2:55 ` Sandy Harris
1 sibling, 1 reply; 8+ messages in thread
From: Herbert Xu @ 2014-11-10 14:05 UTC (permalink / raw)
To: Stephan Mueller; +Cc: linux-kernel, linux-crypto
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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-10 14:05 ` Herbert Xu
@ 2014-11-11 2:06 ` Stephan Mueller
2014-11-11 2:53 ` Herbert Xu
0 siblings, 1 reply; 8+ messages in thread
From: Stephan Mueller @ 2014-11-11 2:06 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-kernel, linux-crypto
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-11 2:06 ` Stephan Mueller
@ 2014-11-11 2:53 ` Herbert Xu
0 siblings, 0 replies; 8+ messages in thread
From: Herbert Xu @ 2014-11-11 2:53 UTC (permalink / raw)
To: Stephan Mueller; +Cc: linux-kernel, linux-crypto
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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-09 22:33 crypto: zeroization of sensitive data in af_alg Stephan Mueller
2014-11-10 14:05 ` Herbert Xu
@ 2014-11-11 2:55 ` Sandy Harris
2014-11-11 4:16 ` Stephan Mueller
1 sibling, 1 reply; 8+ messages in thread
From: Sandy Harris @ 2014-11-11 2:55 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Herbert Xu, LKML, linux-crypto
On Sun, Nov 9, 2014 at 5:33 PM, Stephan Mueller <smueller@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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-11 2:55 ` Sandy Harris
@ 2014-11-11 4:16 ` Stephan Mueller
2014-11-11 4:19 ` Herbert Xu
2014-11-11 9:19 ` Daniel Borkmann
0 siblings, 2 replies; 8+ messages in thread
From: Stephan Mueller @ 2014-11-11 4:16 UTC (permalink / raw)
To: Sandy Harris, Herbert Xu; +Cc: LKML, linux-crypto
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 <smueller@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-11 4:16 ` Stephan Mueller
@ 2014-11-11 4:19 ` Herbert Xu
2014-11-11 9:19 ` Daniel Borkmann
1 sibling, 0 replies; 8+ messages in thread
From: Herbert Xu @ 2014-11-11 4:19 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Sandy Harris, LKML, linux-crypto
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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: crypto: zeroization of sensitive data in af_alg
2014-11-11 4:16 ` Stephan Mueller
2014-11-11 4:19 ` Herbert Xu
@ 2014-11-11 9:19 ` Daniel Borkmann
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2014-11-11 9:19 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Sandy Harris, Herbert Xu, LKML, linux-crypto
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-11-11 9:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-09 22:33 crypto: zeroization of sensitive data in af_alg Stephan Mueller
2014-11-10 14:05 ` Herbert Xu
2014-11-11 2:06 ` Stephan Mueller
2014-11-11 2:53 ` Herbert Xu
2014-11-11 2:55 ` Sandy Harris
2014-11-11 4:16 ` Stephan Mueller
2014-11-11 4:19 ` Herbert Xu
2014-11-11 9:19 ` Daniel Borkmann
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.