Dmitry Eremin-Solenikov dbaryshkov@gmail.com writes:
Hash function GOST R 34.11-94 (gosthash94) in its compression function uses Russian block cipher (GOST 28147-89, Magma). Start separating block cipher code from hash function code. For now there is no public interface for this cipher, it will be added later.
I'm having an initial look at this, with a few comments.
--- /dev/null +++ b/gost28147.c +/*
- A macro that performs a full encryption round of GOST 28147-89.
- Temporary variables tmp assumed and variables r and l for left and right
- blocks.
- */
+#define GOST_ENCRYPT_ROUND(key1, key2, sbox) \
- tmp = (key1) + r; \
- l ^= (sbox)[0*256 + (tmp & 0xff)] ^ (sbox)[1*256 + ((tmp >> 8) & 0xff)] ^ \
- (sbox)[2*256 + ((tmp >> 16) & 0xff)] ^ (sbox)[3*256 + (tmp >> 24)]; \
- tmp = (key2) + l; \
- r ^= (sbox)[0*256 + (tmp & 0xff)] ^ (sbox)[1*256 + ((tmp >> 8) & 0xff)] ^ \
- (sbox)[2*256 + ((tmp >> 16) & 0xff)] ^ (sbox)[3*256 + (tmp >> 24)];
This code is just moved around in this patch, but I'd like to note that it's preferable to always wrap function-like macros like this in do { ... } while (0), and when used terminate with ;. And avoid using surrounding variables; r and l could be macro arguments, and tmp (with some likely unique prefix) could be a local in the do { ... } while block.
--- /dev/null +++ b/gost28147.h @@ -0,0 +1,63 @@
+struct gost28147_param +{
- uint32_t sbox[4*256];
+};
Why change to a flat array, and not keep it as
uint32_t sbox[4][256];
?
+extern const struct gost28147_param gost28147_param_test_3411;
I find "test" in the name a bit odd. Is there a reason for that? And declaration should probably not be in an installed header file, but in gost-internal.h or so.
+/* Internal interface for use by GOST R 34.11-94 */ +void gost28147_encrypt_simple (const uint32_t *key, const uint32_t *sbox,
const uint32_t *in, uint32_t *out);
Same here: if internal, shouldn't be in an installed header file. And "simple" looks a bit odd.
Should the sbox argument be of type const gost28147_param * ?
Regards, /Niels