[dm-crypt] unsafe??? use of memset

[dm-crypt] unsafe??? use of memset

[.. ink ..]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

a lot of people like this one[2] advises against the use of memset to clear
memory but crypsetup seems to
ignore this advice and use memset a lot like in[1].

Any reason why cryptseup is ignoring this advice?

[1]
https://code.google.com/p/cryptsetup/source/browse/lib/tcrypt/tcrypt.c#272
[2]
http://edc.tversu.ru/elib/inf/0088/0596003943_secureprgckbk-chp-13-sect-2.html

[-- Attachment #2: Type: text/html, Size: 646 bytes --]

Re: [dm-crypt] unsafe??? use of memset

[Milan Broz]

On 12/30/2014 02:57 PM, .. ink .. wrote:
> 
> a lot of people like this one[2] advises against the use of memset to clear memory but crypsetup seems to
> ignore this advice and use memset a lot like in[1].
> 
> Any reason why cryptseup is ignoring this advice?

Why ignore? It worked with old compilers (and VC is not the issue here).

This is opensource, so I usually respond with "send a patch" to these messages...

But actually I have patch for that for weeks. I have just another issues which have
unfortunately much higher priority in my life and I am not going commit half-baked patch.

FYI:
I fixed it is kernel dmcrypt, there we can use memzero_explicit()
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=1a71d6ffe18c0d0f03fc8531949cc8ed41d702ee

Cryptsetup will follow (hopefully soon with other fixes).

And it is nothing critical.

There is a nice description of problem
https://cryptocoding.net/index.php/Coding_rules#Prevent_compiler_interference_with_security-critical_operations

Actually I want to replace zero memset with zero it with code used in BLAKE2.
It is simple and should work.

static inline void secure_zero_memory(void *v, size_t n)
{
  volatile uint8_t *p = (volatile uint8_t *)v;
  while(n--) *p++ = 0;
}

Milan

> 
> [1] https://code.google.com/p/cryptsetup/source/browse/lib/tcrypt/tcrypt.c#272
> [2] http://edc.tversu.ru/elib/inf/0088/0596003943_secureprgckbk-chp-13-sect-2.html

Re: [dm-crypt] unsafe??? use of memset

[Arno Wagner]

Interesting question. I think it is not relevant for most
Linux scenarios, as memset() comes precompiled as part of
a binary library, and the compiler has no clue what it 
does and hence cannot optimize it away. 

If memset is compiled together with the code using it, 
this would be a problem, but also one anybody writing
secure code should be aware of. I am not aware of any 
normal Linux scenarios where that could happen.

Still, soemthing low-priority to fix eventually, as 
it cannot be ruled out that it may some day be compiled
in a dangerous fashion or memset() may be made a macro
or some other bizarre circumstances.

BTW, with GCC, there is also the possibility to locally
prohibit optimization with something like:

#pragma GCC push_options
#pragma GCC optimize ("O0")
 code
#pragma GCC pop_options

I needed that some time ago, but do not remember for what.

Anyways, this is an area where recipes do not cut it. For
secure code you have to understand how it gets compiled 
on the specific target platform and what the issues there
are. 

Arno

On Tue, Dec 30, 2014 at 14:57:56 CET, .. ink .. wrote:
> a lot of people like this one[2] advises against the use of memset to clear
> memory but crypsetup seems to
> ignore this advice and use memset a lot like in[1].
> 
> Any reason why cryptseup is ignoring this advice?
> 
> [1]
> https://code.google.com/p/cryptsetup/source/browse/lib/tcrypt/tcrypt.c#272
> [2]
> http://edc.tversu.ru/elib/inf/0088/0596003943_secureprgckbk-chp-13-sect-2.html

> _______________________________________________
> dm-crypt mailing list
> dm-crypt@saout.de
> http://www.saout.de/mailman/listinfo/dm-crypt


-- 
Arno Wagner,     Dr. sc. techn., Dipl. Inform.,    Email: arno@wagner.name
GnuPG: ID: CB5D9718  FP: 12D6 C03B 1B30 33BB 13CF  B774 E35C 5FA1 CB5D 9718
----
A good decision is based on knowledge and not on numbers. -- Plato

If it's in the news, don't worry about it.  The very definition of 
"news" is "something that hardly ever happens." -- Bruce Schneier

Re: [dm-crypt] unsafe??? use of memset

[Arno Wagner]

On Tue, Dec 30, 2014 at 15:26:02 CET, Milan Broz wrote:
> On 12/30/2014 02:57 PM, .. ink .. wrote:
> > 
> > a lot of people like this one[2] advises against the use of memset to clear memory but crypsetup seems to
> > ignore this advice and use memset a lot like in[1].
> > 
> > Any reason why cryptseup is ignoring this advice?
> 
> Why ignore? It worked with old compilers (and VC is not the issue here).
> 
> This is opensource, so I usually respond with "send a patch" to these messages...
> 
> But actually I have patch for that for weeks. I have just another issues which have
> unfortunately much higher priority in my life and I am not going commit half-baked patch.
> 
> FYI:
> I fixed it is kernel dmcrypt, there we can use memzero_explicit()
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=1a71d6ffe18c0d0f03fc8531949cc8ed41d702ee
> 
> Cryptsetup will follow (hopefully soon with other fixes).
> 
> And it is nothing critical.
> 
> There is a nice description of problem
> https://cryptocoding.net/index.php/Coding_rules#Prevent_compiler_interference_with_security-critical_operations

Interessting! So the problem is that memset() may not even be called. 
That would be bad. In that case the compiler would need to know that
there are no volatile variables used inside memset(), which again,
I think it should not be able to on Linux as gcc does not look
at the libraries before linking. Apparently MS Visual C++ 2010
knows more about the libraries than is good for it. 

My take would be that this is a legal optimization (with regard to
the C standard), but one that needs some hidden special treatment
of memset(). Of course I could be wrong.

Arno


> Actually I want to replace zero memset with zero it with code used in BLAKE2.
> It is simple and should work.
> 
> static inline void secure_zero_memory(void *v, size_t n)
> {
>   volatile uint8_t *p = (volatile uint8_t *)v;
>   while(n--) *p++ = 0;
> }
> 
> Milan
> 
> > 
> > [1] https://code.google.com/p/cryptsetup/source/browse/lib/tcrypt/tcrypt.c#272
> > [2] http://edc.tversu.ru/elib/inf/0088/0596003943_secureprgckbk-chp-13-sect-2.html
> _______________________________________________
> dm-crypt mailing list
> dm-crypt@saout.de
> http://www.saout.de/mailman/listinfo/dm-crypt

-- 
Arno Wagner,     Dr. sc. techn., Dipl. Inform.,    Email: arno@wagner.name
GnuPG: ID: CB5D9718  FP: 12D6 C03B 1B30 33BB 13CF  B774 E35C 5FA1 CB5D 9718
----
A good decision is based on knowledge and not on numbers. -- Plato

If it's in the news, don't worry about it.  The very definition of 
"news" is "something that hardly ever happens." -- Bruce Schneier