Hello,
вт, 9 июл. 2019 г. в 01:17, Niels Möller nisse@lysator.liu.se:
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.
Fine, I will check if this doesn't lower the performance and redo the code.
--- /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];
?
I don't remember. I did this long ago. I'll try changing this back.
+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.
It is called 'test' because it was declared so in the standard itself (for testing purposes only). Also see RFC 4357, Section 11.2 (id-GostR3411-94-TestParamSet).
+/* 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.
gost-internal.h sounds like a good idea, thank you.
Should the sbox argument be of type const gost28147_param * ?
No. gost28147_param is a high-level item, comprising of S-BOX, requirement to use key meshing. They are exported for external usage (via pointers). gost28147_encrypt_simple is an internal API which uses S-BOX directly, as it doesn't use any other 'param' part.