0

There are two elements in Kuznyechik source code from VeraCrypt source code: https://github.com/veracrypt/VeraCrypt/blob/master/src/Crypto/kuznyechik.c#L2271-L2272

uint64 x1 = *(const uint64*)in;
uint64 x2 = *(((const uint64*)in)+1);

They are processed in the Kuznyechik S-Box like this:

#define LS(x1,x2,t1,t2) { \
        t1 = T[0][(byte)(x1)][0] ^ T[1][(byte)(x1 >> 8)][0] ^ T[2][(byte)(x1 >> 16)][0] ^ T[3][(byte)(x1 >> 24)][0] ^ T[4][(byte)(x1 >> 32)][0] ^ T[5][(byte)(x1 >> 40)][0] ^ \
            T[6][(byte)(x1 >> 48)][0] ^ T[7][(byte)(x1 >> 56)][0] ^ T[8][(byte)(x2)][0] ^ T[9][(byte)(x2 >> 8)][0] ^ T[10][(byte)(x2 >> 16)][0] ^ T[11][(byte)(x2 >> 24)][0] ^ \
            T[12][(byte)(x2 >> 32)][0] ^ T[13][(byte)(x2 >> 40)][0] ^ T[14][(byte)(x2 >> 48)][0] ^ T[15][(byte)(x2 >> 56)][0]; \
        t2 = T[0][(byte)(x1)][1] ^ T[1][(byte)(x1 >> 8)][1] ^ T[2][(byte)(x1 >> 16)][1] ^ T[3][(byte)(x1 >> 24)][1] ^ T[4][(byte)(x1 >> 32)][1] ^ T[5][(byte)(x1 >> 40)][1] ^ \
            T[6][(byte)(x1 >> 48)][1] ^ T[7][(byte)(x1 >> 56)][1] ^ T[8][(byte)(x2)][1] ^ T[9][(byte)(x2 >> 8)][1] ^ T[10][(byte)(x2 >> 16)][1] ^ T[11][(byte)(x2 >> 24)][1] ^ \
            T[12][(byte)(x2 >> 32)][1] ^ T[13][(byte)(x2 >> 40)][1] ^ T[14][(byte)(x2 >> 48)][1] ^ T[15][(byte)(x2 >> 56)][1]; \
    }

/\ https://github.com/veracrypt/VeraCrypt/blob/master/src/Crypto/kuznyechik.c#L2147-L2152

What does this mean? Does it mean that x1 will process the first to eighth byte in t1 and will do the same again but from eighth to first byte (in inverse order)? am I right?

phantomcraft
  • 125
  • 4
  • 2
    You shouldn't write slop like `*(const uint64*)in;`. This can cause problems with alignment and strict aliasing both. – Lundin Apr 07 '22 at 13:54
  • @Lundin That's not a helpful comment on a "what does this do" question. OP did not write this code. – zwol Apr 07 '22 at 14:42
  • It looks like the code is basically manipulating 128-bit numbers. `x1` and `x2` are two 64-bit halves of a 128-bit number, as are `t1` and `t2`. – Ian Abbott Apr 07 '22 at 14:58
  • 1
    @zwol It is helpful since it means that the question is actually: "What does this code with undefined behavior do". It's also helpful to point out that the OP might be peeking into a badly written code base. If that wasn't already blatantly evident by the quoted macro. – Lundin Apr 07 '22 at 18:49
  • @Lundin Yes, but what you said communicates none of those things. (_I_ know what you meant, but only because I have seen you complain about this kind of code before.) – zwol Apr 07 '22 at 19:01

1 Answers1

1

Looking at somewhat more of the code than what you posted, I see that in is a function parameter declared as (a typedef for) const char *. Given that, the first fragment you posted could have been written more readably as

const uint64_t *in_qword = (const uint64_t *)in;
uint64_t x1 = in_qword[0];
uint64_t x2 = in_qword[1];

(This program's uint64 and the standard uint64_t appear to be the same thing, but I did not dig through the maze of #ifdefs in this program's header files carefully enough to be sure of that. It doesn't actually matter for the rest of what I'm going to say.)

Rewritten this way, we can see that we have a pointer to an array of char and the program is attempting to access the first several chars of that array as-if it had been an array of type uint64_t all along. This is a relatively common thing to see in cryptography code, but the C language does not actually allow you to do this. Both the original code and my rewrite have what is formally known as "undefined behavior". In my somewhat checkered experience, code like this will probably behave as intended as long as you don't try to use link-time optimization, but that is something you are getting away with, not something you're allowed to rely on.

(OK, how should you take the first sizeof(uint64_t) bytes of a char array and load them into a uint64_t as a machine-endian integer? The only strictly conforming way to do it is

memcpy(&x1, &in[0], sizeof(uint64_t));

but this doesn't give the "yes it compiles to one load instruction" performance guarantee that people usually want in low-level code like this. The C language doesn't offer any alternative, which is why people keep using "type punning" even though it's incorrect.)

The second fragment you posted appears to be doing byte shuffling and S-box lookups in typical illegible fashion for a cryptographic primitive. I cannot tell whether it is correct C in isolation and I am not qualified to say whether the cipher is any good.

zwol
  • 135,547
  • 38
  • 252
  • 361
  • *this doesn't give the "yes it compiles to one load instruction" performance guarantee that people usually want in low-level code like this* If they're that concerned with performance, they'd observe that good optimizing compilers do optimize `memcpy()` to equivalent code when safe. *which is why people keep using "type punning" even though it's incorrect* In my experience it's usually ignorance of the C standard's strict aliasing and alignment rules, augmented in a lot of cases by the person writing the code confusing, "But it works!" with the truth: "You haven't observed to fail. Yet." – Andrew Henle Apr 07 '22 at 22:14
  • "code like this will probably behave as intended" Depends on what you pass. If it's the start address of a byte array located in `.data`, then yeah it will probably work unless optimizations are too aggressive. But if it's some byte array on the stack, all bets are off, alignment is a real thing. – Lundin Apr 08 '22 at 06:17