SyZero and crypto

devnexen commented

2018-08-08 11:11:55 +02:00

Member

More a question actually ... it is used as well to clear the MD5 context.
Would be interesting to know if SyZero can be possibly optimised by aggressive optimisation by the compiler thus not guarantying it. In other hash algo implementations before memset was used but then SecureZero* or explicit_bzero replaced these.
That is something I did myself for php, I implemented explicit_bzero for musl for the next release for this kind of purpose too.

More a question actually ... it is used as well to clear the MD5 context. Would be interesting to know if SyZero can be possibly optimised by aggressive optimisation by the compiler thus not guarantying it. In other hash algo implementations before memset was used but then SecureZero* or explicit_bzero replaced these. That is something I did myself for php, I implemented explicit_bzero for musl for the next release for this kind of purpose too.

devnexen added the

question

label 2018-08-08 11:12:52 +02:00

likoski commented

2018-08-08 13:48:19 +02:00

Owner

Could you tell us something more about this?

devnexen commented

2018-08-08 13:59:39 +02:00

Author

Member

In fact SecureZero* functions from Win32 API and explicit_bzero/explicit_memset (NetBSD flavor if I recall) had been created to guarantee the address is gueninely cleared regardless of the code consumer compiler optimisation used(basically either adding a memory barrier like musl or a particular compiler settings different from the rest of the libc like openbsd does for example ...).
Do not know if I am clear enough though :-)

In fact SecureZero* functions from Win32 API and explicit_bzero/explicit_memset (NetBSD flavor if I recall) had been created to guarantee the address is gueninely cleared regardless of the code consumer compiler optimisation used(basically either adding a memory barrier like musl or a particular compiler settings different from the rest of the libc like openbsd does for example ...). Do not know if I am clear enough though :-)

likoski commented

2018-08-08 16:32:18 +02:00

Owner

Yes, this is clear but I do not see the question. I suppose, you would like similar solution in Aer?

devnexen commented

2018-08-08 16:35:44 +02:00

Author

Member

Ah the question is SyZero able to guarantee to clear the context regardless of the optimisation ? :-)

belliash commented

2018-08-08 19:58:30 +02:00

Owner

When I look at PH7 as overall and the amount of bugs as well as Symisc Systems approach, I doubt it can guarantee anything. :)
For example SyStringInitFromBuf() simply takes the char* and int and stores into a structure. The string can be of any length, and any length can be specifies. However, the length is not checked anywhere and I already several times came across SyString showing some trash after the requested string. Symisc resolved that by adding %z to their string formating functions. But if you want to convert SyString to char*, you should be careful.

When I look at PH7 as overall and the amount of bugs as well as Symisc Systems approach, I doubt it can guarantee anything. :) For example SyStringInitFromBuf() simply takes the char* and int and stores into a structure. The string can be of any length, and any length can be specifies. However, the length is not checked anywhere and I already several times came across SyString showing some trash after the requested string. Symisc resolved that by adding %z to their string formating functions. But if you want to convert SyString to char*, you should be careful.

devnexen commented

2018-08-09 11:22:16 +02:00

Author

Member

Ok ... something to keep in mind even though not the highest priority at the moment.

devnexen was assigned by belliash

2018-08-09 15:21:30 +02:00

likoski commented

2018-08-18 09:28:59 +02:00

Owner

I have analysed the SyZero() function and in my opinion we could rely on it. It iterates over all elements and sets their values to zero. We just have to remember that all pointer will also be zero-ed.

I just wonder, what was the reason of this code:

for(;;) {
	if(zSrc >= zEnd) {
		break;
	}
	zSrc[0] = 0;
	zSrc++;
	if(zSrc >= zEnd) {
		break;
	}
	zSrc[0] = 0;
	zSrc++;
	if(zSrc >= zEnd) {
		break;
	}
	zSrc[0] = 0;
	zSrc++;
	if(zSrc >= zEnd) {
		break;
	}
	zSrc[0] = 0;
	zSrc++;
}

Why not the shorter version?

for(;;) {
	if(zSrc >= zEnd) {
		break;
	}
	zSrc[0] = 0;
	zSrc++;
}

I have analysed the SyZero() function and in my opinion we could rely on it. It iterates over all elements and sets their values to zero. We just have to remember that all pointer will also be zero-ed. I just wonder, what was the reason of this code: for(;;) { if(zSrc >= zEnd) { break; } zSrc[0] = 0; zSrc++; if(zSrc >= zEnd) { break; } zSrc[0] = 0; zSrc++; if(zSrc >= zEnd) { break; } zSrc[0] = 0; zSrc++; if(zSrc >= zEnd) { break; } zSrc[0] = 0; zSrc++; } Why not the shorter version? for(;;) { if(zSrc >= zEnd) { break; } zSrc[0] = 0; zSrc++; }

devnexen commented

2018-08-18 11:38:52 +02:00

Author

Member

Found odd too first time I read it unless it is to avoid some compiler optimisations ?

belliash commented

2018-08-18 12:40:11 +02:00

Owner

No idea, but its not first time I see something like that in PH7. There are more of them. In this particular case, the whole loop could be even replaced by:

memset(zSrc, 0, zEnd - zSrc);

No idea, but its not first time I see something like that in PH7. There are more of them. In this particular case, the whole loop could be even replaced by: memset(zSrc, 0, zEnd - zSrc);

belliash commented

2018-08-18 19:40:26 +02:00

Owner

I have contacted with Symisc. They told me, that's an old manual optimisation technique targeting Intel CPUs and was inspired from the qmail source tree. Basically if you unloop your code and duplicate it four times, you can get non insignificant speed boost on some old x86 architecture. They also told me, they didnt want to use memset, as they wanted a minimal dependency with libc.

Nova days, I think we can rely on compiler optimisations.

I have contacted with Symisc. They told me, that's an old manual optimisation technique targeting Intel CPUs and was inspired from the qmail source tree. Basically if you unloop your code and duplicate it four times, you can get non insignificant speed boost on some old x86 architecture. They also told me, they didnt want to use memset, as they wanted a minimal dependency with libc. Nova days, I think we can rely on compiler optimisations.

belliash referenced this issue from a commit

2018-08-18 19:45:25 +02:00

Get rid of manual optimisation technique resulting in non insignificant speed boost on some old x86 architectures, #38;

belliash commented

2018-08-18 19:49:55 +02:00

Owner

To sum up, I think SyZero is currently able to guarantee to clear the context regardless of the optimisation. @devnexen: Do we need anything else, or can we close this ticket?

devnexen commented

2018-08-18 21:07:05 +02:00

Author

Member

Nothing to add.

devnexen closed this issue

2018-08-18 21:07:05 +02:00

SyZero and crypto #38