Re: rsaenh: Remove dead assignments (Clang)

[Nikolay Sivov]

On 30.11.2015 0:31, André Hentschel wrote:
> Signed-off-by: André Hentschel
>
> dlls/rsaenh/sha2.c | 21 ---------------------
>   1 file changed, 21 deletions(-)
>
> diff --git a/dlls/rsaenh/sha2.c b/dlls/rsaenh/sha2.c
> index 76e7184..71ebf23 100644
> --- a/dlls/rsaenh/sha2.c
> +++ b/dlls/rsaenh/sha2.c
> @@ -369,9 +369,6 @@ void SHA256_Transform(SHA256_CTX* context, const sha2_word32* data) {
>    context->state[5] += f;
>    context->state[6] += g;
>    context->state[7] += h;
> -
> -
>
> /* Clean up */
> -
>
> a = b = c = d = e = f = g = h = T1 = 0;
> }
>
> #else /* SHA2_UNROLL_TRANSFORM */
>
> @@ -449,9 +446,6 @@ void SHA256_Transform(SHA256_CTX* context, const sha2_word32* data) {
>    context->state[5] += f;
>    context->state[6] += g;
>    context->state[7] += h;
> -
> -
>
> /* Clean up */
> -
>
> a = b = c = d = e = f = g = h = T1 = T2 = 0;
> }
>
> #endif /* SHA2_UNROLL_TRANSFORM */
>
> @@ -483,8 +477,6 @@ void SHA256_Update(SHA256_CTX* context, const sha2_byte *data, size_t len) {
>    		/* The buffer is not yet full */
>    		MEMCPY_BCOPY(&context->buffer[usedspace], data, len);
>    		context->bitcount += len << 3;
> -
> 	/* Clean up: */
> -
> 	usedspace = freespace = 0;
> return;
> }
> }
>
> @@ -500,8 +492,6 @@ void SHA256_Update(SHA256_CTX* context, const sha2_byte *data, size_t len) {
>    	MEMCPY_BCOPY(context->buffer, data, len);
>    	context->bitcount += len << 3;
>    }
> -
>
> /* Clean up: */
> -
>
> usedspace = freespace = 0;
> }
>
> void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
>
> @@ -564,7 +554,6 @@ void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
>
> /* Clean up state data: */
>    MEMSET_BZERO(context, sizeof(*context));
> -
>
> usedspace = 0;
> }
>
> char *SHA256_End(SHA256_CTX* context, char buffer[]) {
>
> @@ -693,9 +682,6 @@ void SHA512_Transform(SHA512_CTX* context, const sha2_word64* data) {
>    context->state[5] += f;
>    context->state[6] += g;
>    context->state[7] += h;
> -
> -
>
> /* Clean up */
> -
>
> a = b = c = d = e = f = g = h = T1 = 0;
> }
>
> #else /* SHA2_UNROLL_TRANSFORM */
>
> @@ -771,9 +757,6 @@ void SHA512_Transform(SHA512_CTX* context, const sha2_word64* data) {
>    context->state[5] += f;
>    context->state[6] += g;
>    context->state[7] += h;
> -
> -
>
> /* Clean up */
> -
>
> a = b = c = d = e = f = g = h = T1 = T2 = 0;
> }
>
> #endif /* SHA2_UNROLL_TRANSFORM */
>
> @@ -805,8 +788,6 @@ void SHA512_Update(SHA512_CTX* context, const sha2_byte *data, size_t len) {
>    		/* The buffer is not yet full */
>    		MEMCPY_BCOPY(&context->buffer[usedspace], data, len);
>    		ADDINC128(context->bitcount, len << 3);
> -
> 	/* Clean up: */
> -
> 	usedspace = freespace = 0;
> return;
> }
> }
>
> @@ -822,8 +803,6 @@ void SHA512_Update(SHA512_CTX* context, const sha2_byte *data, size_t len) {
>    	MEMCPY_BCOPY(context->buffer, data, len);
>    	ADDINC128(context->bitcount, len << 3);
>    }
> -
>
> /* Clean up: */
> -
>
> usedspace = freespace = 0;
> }
>
> void SHA512_Last(SHA512_CTX* context) {

This and advapi32 looks like was done on purpose. Why are you removing that?

rsaenh: Remove dead assignments (Clang)

[Marcus Meissner]

On Mon, Nov 30, 2015 at 01:39:09AM +0300, Nikolay Sivov wrote:
> On 30.11.2015 0:31, André Hentschel wrote:
> > Signed-off-by: André Hentschel
> >
> > dlls/rsaenh/sha2.c | 21 ---------------------
> >  1 file changed, 21 deletions(-)
> >
> > diff --git a/dlls/rsaenh/sha2.c b/dlls/rsaenh/sha2.c
> > index 76e7184..71ebf23 100644
> > --- a/dlls/rsaenh/sha2.c
> > +++ b/dlls/rsaenh/sha2.c
> > @@ -369,9 +369,6 @@ void SHA256_Transform(SHA256_CTX* context, const sha2_word32* data) {
> >    context->state[5] += f;
> >    context->state[6] += g;
> >    context->state[7] += h;
> > -
> > - /* Clean up */
> > - a = b = c = d = e = f = g = h = T1 = 0;
> >
> > }
> >
> > #else /* SHA2_UNROLL_TRANSFORM */
> > @@ -449,9 +446,6 @@ void SHA256_Transform(SHA256_CTX* context, const sha2_word32* data) {
> >    context->state[5] += f;
> >    context->state[6] += g;
> >    context->state[7] += h;
> > -
> > - /* Clean up */
> > - a = b = c = d = e = f = g = h = T1 = T2 = 0;
> >
> > }
> >
> > #endif /* SHA2_UNROLL_TRANSFORM */
> > @@ -483,8 +477,6 @@ void SHA256_Update(SHA256_CTX* context, const sha2_byte *data, size_t len) {
> >    		/* The buffer is not yet full */
> >    		MEMCPY_BCOPY(&context->buffer[usedspace], data, len);
> >    		context->bitcount += len << 3;
> > -
> > 	/* Clean up: */
> > -
> > 	usedspace = freespace = 0;
> > return;
> > }
> > }
> >
> > @@ -500,8 +492,6 @@ void SHA256_Update(SHA256_CTX* context, const sha2_byte *data, size_t len) {
> >    	MEMCPY_BCOPY(context->buffer, data, len);
> >    	context->bitcount += len << 3;
> >    }
> > - /* Clean up: */
> > - usedspace = freespace = 0;
> >
> > }
> >
> > void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
> > @@ -564,7 +554,6 @@ void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
> >
> > /* Clean up state data: */
> >    MEMSET_BZERO(context, sizeof(*context));
> > - usedspace = 0;
> >
> > }
> >
> > char *SHA256_End(SHA256_CTX* context, char buffer[]) {
> > @@ -693,9 +682,6 @@ void SHA512_Transform(SHA512_CTX* context, const sha2_word64* data) {
> >    context->state[5] += f;
> >    context->state[6] += g;
> >    context->state[7] += h;
> > -
> > - /* Clean up */
> > - a = b = c = d = e = f = g = h = T1 = 0;
> >
> > }
> >
> > #else /* SHA2_UNROLL_TRANSFORM */
> > @@ -771,9 +757,6 @@ void SHA512_Transform(SHA512_CTX* context, const sha2_word64* data) {
> >    context->state[5] += f;
> >    context->state[6] += g;
> >    context->state[7] += h;
> > -
> > - /* Clean up */
> > - a = b = c = d = e = f = g = h = T1 = T2 = 0;
> >
> > }
> >
> > #endif /* SHA2_UNROLL_TRANSFORM */
> > @@ -805,8 +788,6 @@ void SHA512_Update(SHA512_CTX* context, const sha2_byte *data, size_t len) {
> >    		/* The buffer is not yet full */
> >    		MEMCPY_BCOPY(&context->buffer[usedspace], data, len);
> >    		ADDINC128(context->bitcount, len << 3);
> > -
> > 	/* Clean up: */
> > -
> > 	usedspace = freespace = 0;
> > return;
> > }
> > }
> >
> > @@ -822,8 +803,6 @@ void SHA512_Update(SHA512_CTX* context, const sha2_byte *data, size_t len) {
> >    	MEMCPY_BCOPY(context->buffer, data, len);
> >    	ADDINC128(context->bitcount, len << 3);
> >    }
> > - /* Clean up: */
> > - usedspace = freespace = 0;
> >
> > }
> >
> > void SHA512_Last(SHA512_CTX* context) {
>
> This and advapi32 looks like was done on purpose. Why are you removing that?

The usual idea in cryptography is to zeroize temporary components.

The compiler will optimize away however if its just stack space :(

Ciao, Marcus

rsaenh: Remove dead assignments (Clang)

[André Hentschel]

Am 30.11.2015 um 09:51 schrieb Marcus Meissner:
> On Mon, Nov 30, 2015 at 01:39:09AM +0300, Nikolay Sivov wrote:
> > On 30.11.2015 0:31, André Hentschel wrote:
> > > Signed-off-by: André Hentschel
> > >
> > > dlls/rsaenh/sha2.c | 21 ---------------------
> > >  1 file changed, 21 deletions(-)
> >
> > This and advapi32 looks like was done on purpose. Why are you removing that?
>
> The usual idea in cryptography is to zeroize temporary components.
>
> The compiler will optimize away however if its just stack space :(

That's exactly the reason, compilers would maybe only care about it with -O0

rsaenh: Remove dead assignments (Clang)

[Vincent Povirk]

> > The usual idea in cryptography is to zeroize temporary components.
> >
> > The compiler will optimize away however if its just stack space :(
>
> That's exactly the reason, compilers would maybe only care about it with -O0

FWIW, SecureZeroMemory exists to prevent that optimization.