#173 - SHA3 - gh-libtom/libtomcrypt - Forgejo: Beyond coding. We Forge.

karel-m commented

2017-03-22 15:27:48 -05:00

(Migrated from github.com)

SHAKE128 and SHAKE256 need to be added - I only do not know yet how to do it (they do not fit well to our hash machinery).

The implementation is based on https://github.com/brainhub/SHA3IUF

SHAKE128 and SHAKE256 need to be added - I only do not know yet how to do it (they do not fit well to our hash machinery). The implementation is based on https://github.com/brainhub/SHA3IUF

karel-m commented

2017-03-22 19:25:56 -05:00

(Migrated from github.com)

I am extremely strongly against creating dependency on stdint.h.

I lost this battle in libtommath and the result is that I have to keep a specially patched tommath.h for my perl bindings which is a pain for me. Frankly we did not gain anything in libtommath by starting using stdint.h.

And please do not give me those stories about how old is C99. It was a week ago when I was trying to improve libtomcrypt's Visual C builds. And guess what, on Visual Studio 2008 Express there is no stdint.h so tommath.h hit me again.

In short: either me or stdint.h 😃

**I am extremely strongly against creating dependency on `stdint.h`.** I lost this battle in libtommath and the result is that I have to keep a specially patched `tommath.h` for my perl bindings which is a pain for me. Frankly we did not gain anything in libtommath by starting using `stdint.h`. And please do not give me those stories about how old is C99. It was a week ago when I was trying to improve libtomcrypt's Visual C builds. And guess what, on Visual Studio 2008 Express there is no `stdint.h` so `tommath.h` hit me again. In short: either `me` or `stdint.h` 😃

rofl0r commented

2017-03-22 21:03:30 -05:00

(Migrated from github.com)

using size_t does not necessarily mean including stdint.h. since you're a fan of ifdef hacks over configure tests you could hardcode a list of CPP defines which define HAVE_STDINT_H and only use it there, for the other platforms fall back to typedefing it to unsigned long, or unsigned long long on windows64 bit.

karel-m commented

2017-03-23 02:56:15 -05:00

(Migrated from github.com)

Let's move size_t discussion to #175 so that it does not block this PR

Let's move `size_t` discussion to #175 so that it does not block this PR

sjaeckel commented

2017-03-23 17:41:36 -05:00

(Migrated from github.com)

Regarding 4abbf45731c235c973da9a1ae349bc788eb232ca wouldn't it make sense to have a separate sha3_shake hash descriptor so we can re-use the hash helpers?

Regarding 4abbf45731c235c973da9a1ae349bc788eb232ca wouldn't it make sense to have a separate `sha3_shake` hash descriptor so we can re-use the hash helpers?

karel-m commented

2017-03-24 01:23:09 -05:00

(Migrated from github.com)

unfortunately sha3_shake_done does not fit into ltc_hash_descriptor because it is missing the outlen (SHAKE functions are designed in a way that they give the output as long as you ask.

int (*done)(hash_state *hash, unsigned char *out);

unfortunately `sha3_shake_done` does not fit into `ltc_hash_descriptor` because it is missing the `outlen` (SHAKE functions are designed in a way that they give the output as long as you ask. ```C int (*done)(hash_state *hash, unsigned char *out); ```

karel-m commented

2017-03-24 03:47:45 -05:00

(Migrated from github.com)

Kind of outlen inconsistency we have here:

int hash_memory(int hash, const unsigned char *in,  unsigned long inlen, unsigned char *out, unsigned long *outlen);
int sha3_shake_memory(int num, const unsigned char *in, unsigned long inlen, unsigned char *out, unsigned long outlen);

The thing is that hash_memory gives you the output as long as the hashsize is, whereas sha3_shake_memory gives you the output as long as you have requested by outlen param. So these functions are a bit different in their nature, sha3_shake_memory is more like hkdf or prng->read where we use outlen not *outlen.

But providing that we have a separate outlen related issue #176 I propose to discuss/solve it rather there than to block this PR. When we decide to change outlen to *outlen it is trivial to change the implementation.

Kind of `outlen` inconsistency we have here: ```C int hash_memory(int hash, const unsigned char *in, unsigned long inlen, unsigned char *out, unsigned long *outlen); int sha3_shake_memory(int num, const unsigned char *in, unsigned long inlen, unsigned char *out, unsigned long outlen); ``` The thing is that `hash_memory` gives you the output as long as the `hashsize` is, whereas `sha3_shake_memory` gives you the output as long as you have requested by `outlen` param. So these functions are a bit different in their nature, `sha3_shake_memory` is more like `hkdf` or `prng->read` where we use `outlen` not `*outlen`. But providing that we have a separate outlen related issue #176 I propose to discuss/solve it rather there than to block this PR. When we decide to change `outlen` to `*outlen` it is trivial to change the implementation.

coveralls commented

2017-03-26 16:25:55 -05:00

(Migrated from github.com)

Coverage increased (+0.06%) to 87.475% when pulling 95c94bf317b05913a98fbef81e79afe4b84681b8 on pr/sha3 into 7885c91af3 on develop.

[![Coverage Status](https://coveralls.io/builds/10779971/badge)](https://coveralls.io/builds/10779971) Coverage increased (+0.06%) to 87.475% when pulling **95c94bf317b05913a98fbef81e79afe4b84681b8 on pr/sha3** into **7885c91af3a5fb653e2f2640afb2724c24314df1 on develop**.

karel-m commented

2017-03-26 16:42:46 -05:00

(Migrated from github.com)

Rebased & squashed into 1 commit, travis-ci all green. I propose merging to develop.

sjaeckel commented

2017-03-28 14:42:29 -05:00

(Migrated from github.com)

I'd prefer to have sha3_shake_memory() already with *outlen then it's fine by me to merge

I'd prefer to have `sha3_shake_memory()` already with `*outlen` then it's fine by me to merge

karel-m commented

2017-03-29 02:03:11 -05:00

(Migrated from github.com)

sha3_shake_memory changed

Should I also change int sha3_shake_done(hash_state *md, unsigned char *out, unsigned long outlen) to *outlen? By "normal" hashes we have no outlen - like int sha3_done(hash_state *md, unsigned char *hash)

`sha3_shake_memory` changed Should I also change `int sha3_shake_done(hash_state *md, unsigned char *out, unsigned long outlen)` to `*outlen`? By "normal" hashes we have no outlen - like `int sha3_done(hash_state *md, unsigned char *hash)`

coveralls commented

2017-03-29 02:26:43 -05:00

(Migrated from github.com)

Coverage increased (+0.07%) to 87.478% when pulling 9c92c0a4c45ae640e99c9cf8f467a5fd32e12870 on pr/sha3 into 7885c91af3 on develop.

[![Coverage Status](https://coveralls.io/builds/10823244/badge)](https://coveralls.io/builds/10823244) Coverage increased (+0.07%) to 87.478% when pulling **9c92c0a4c45ae640e99c9cf8f467a5fd32e12870 on pr/sha3** into **7885c91af3a5fb653e2f2640afb2724c24314df1 on develop**.

sjaeckel commented

2017-03-29 06:01:53 -05:00

(Migrated from github.com)

Should I also change int sha3_shake_done(hash_state *md, unsigned char *out, unsigned long outlen) to *outlen? By "normal" hashes we have no outlen - like int sha3_done(hash_state *md, unsigned char *hash)

I also thought about that, but as there's no reference I'd say we keep it like that.

btw. I also had the idea why can't we provide a sha3_shake_set_outlen(hash_state *md, unsigned long outlen), then we could have sha3_shake_done(hash_state *md, unsigned char *hash) :)

but it was just an idea and as there's no way to properly integrate this in the 'hashing framework' I think it doesn't make sense to squeeze the sha3 shake API in there

> Should I also change `int sha3_shake_done(hash_state *md, unsigned char *out, unsigned long outlen)` to `*outlen`? By "normal" hashes we have no outlen - like `int sha3_done(hash_state *md, unsigned char *hash)` I also thought about that, but as there's no reference I'd say we keep it like that. btw. I also had the idea why can't we provide a `sha3_shake_set_outlen(hash_state *md, unsigned long outlen)`, then we could have `sha3_shake_done(hash_state *md, unsigned char *hash)` :) but it was just an idea and as there's no way to properly integrate this in the 'hashing framework' I think it doesn't make sense to squeeze the sha3 shake API in there

coveralls commented

2017-03-29 06:57:10 -05:00

(Migrated from github.com)

Coverage increased (+0.07%) to 87.478% when pulling c6a0362c88 on pr/sha3 into 1b81848576 on develop.

[![Coverage Status](https://coveralls.io/builds/10826665/badge)](https://coveralls.io/builds/10826665) Coverage increased (+0.07%) to 87.478% when pulling **c6a0362c88131ce5ccf269b6b97e3f45f979073f on pr/sha3** into **1b81848576c1064191930a07f67b4c092fe3069f on develop**.

Rows
Columns

SHA3 #173