4 ms·

#define FOR(i, start, end) for (size_t (i) = (start); (i) < (end); (i)++) yuck EDIT: Also, what the hell is this macro defined in the middle of a function and

#define FOR(i, start, end) for (size_t (i) = (start); (i) < (end); (i)++) yuck EDIT: Also, what the hell is this macro defined in the middle of a function and then only used twice: https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L1091 https://github.com/LoupVaillant/Monocypher/blob/master/src/m... Why do you need this function for "constant time comparison to 0": https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L68 https://github.com/LoupVaillant/Monocypher/blob/master/src/m... static int neq0(u64 diff) { // constant time comparison to zero // return diff != 0 ? -1 : 0 u64 half = (diff >> 32) | ((u32)diff); return (1 & ((half - 1) >> 32)) - 1; } I am incredibly skeptical that you've done better than the compiler with this bit twiddling stuff. You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right?

lmz 9y ago
> You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right? But will the 1-2 instruction compare be constant time?
- lsiebert 9y ago
  constant time comparison in crypto is potentially important because of timing attacks to gain information. Haven't looked at the code here to see if that's the case, nor am I a crypto expert, but see https://en.wikipedia.org/wiki/Timing_attack https://en.wikipedia.org/wiki/Timing_attack
  - lmz 9y ago
    I am aware of that. I'm not sure about the poster claiming it reduces to "1-2 instructions".
- loup-vaillant 9y ago
  They likely won't. They may compile into a conditional branch, which because of cache and pipeline invalidation do not run in constant time.
bri3d 9y ago
This isn't an effort to beat the compiler on speed or optimization - rather, the opposite. It's an effort to produce a method which executes in the exact same amount of time for any input - whether or not it is zero. This is key in implementing crypto while avoiding side-channel timing attacks and this isn't a particularly uncommon way to do so. Now, with that being said, this strikes me as a domain issue: the OP seems well-versed in crypto and foundation / backend code, which has relatively constrained input and output behaviors and is testable using smart reasoning and fuzzing. This is somewhat different from front-end or user-facing code, which involves handling epic quantities of mutable state and requires validation of a feature end-to-end. I suspect OP writes great, "bug-free" code in the backend / systems domain, but I don't think their testing practice or anecdotes are applicable across the industry.
- bonesss 9y ago
  > I suspect OP writes great, "bug-free" code in the backend / systems domain, but I don't think their testing practice or anecdotes are applicable across the industry. I'll grant OP a Magic Cloak of Error Abolishment and an enchanted ring with +4 INT, +4 WIS, and +3 VARIABLE-NAMING... Regardless, what's the very first thing I'm gonna do when I inherit his project for support or further development? Start ripping it apart to add tests so that I can verify the behaviour of the system pre, and post, change. If that code base was tested before, even poorly, that means extending the tests and improving their web of assertions and I will be making positive changes to the code within hours. If that code base is untested then my maintenance activities will involve unsupported refactoring and WAGs about system behaviour once the new tests come online. Days of work will hamper the start of maintenance activities. From experience, it will also likley mean finding a bunch of previously hidden issues, bugs, and suspect behaviour that has to be re-analyzed and re-tested before maintenance work can begin in earnest. A strong feeling that something is "bug-free" does not tell me that everything works unchanged on a new platform/OS/architecture, or let me throw in a newly conceived corner case, or check some environmental oddity... If we start out with the premise that systems spend most of their lifetime in maintenance, preparing the system for maintenance activities and trying to eliminate risk in that delicate window make a lot of sense. Not for a lone developer who has a bunch of domain knowledge in their heads, but for teams where new individuals are exposed to new domains and code at the same time.
loup-vaillant 9y ago
The FOR macro is more readable than the original C for loops. Given their sheer number in this code, I think this is worth the 10 seconds learning curve. I don't use that when I work with others, though. The "used twice" macro makes clear this is the exact same code. Would be hard to make sure it is otherwise. Easier to review that way (carry code is a nightmare to test and review). The constant time comparison is necessary to ensure timing attacks cannot happen. The arithmetic trick may be a tad slower than a conditional branch, but its timings are consistent. Note: the compiler is still allowed to just use a branch, but in practice they don't.

lmz 9y ago

> You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right? But will the 1-2 instruction compare be constant time?

lsiebert 9y ago
constant time comparison in crypto is potentially important because of timing attacks to gain information. Haven't looked at the code here to see if that's the case, nor am I a crypto expert, but see https://en.wikipedia.org/wiki/Timing_attack https://en.wikipedia.org/wiki/Timing_attack
- lmz 9y ago
  I am aware of that. I'm not sure about the poster claiming it reduces to "1-2 instructions".
loup-vaillant 9y ago
They likely won't. They may compile into a conditional branch, which because of cache and pipeline invalidation do not run in constant time.

bri3d 9y ago

This isn't an effort to beat the compiler on speed or optimization - rather, the opposite. It's an effort to produce a method which executes in the exact same amount of time for any input - whether or not it is zero. This is key in implementing crypto while avoiding side-channel timing attacks and this isn't a particularly uncommon way to do so. Now, with that being said, this strikes me as a domain issue: the OP seems well-versed in crypto and foundation / backend code, which has relatively constrained input and output behaviors and is testable using smart reasoning and fuzzing. This is somewhat different from front-end or user-facing code, which involves handling epic quantities of mutable state and requires validation of a feature end-to-end. I suspect OP writes great, "bug-free" code in the backend / systems domain, but I don't think their testing practice or anecdotes are applicable across the industry.

bonesss 9y ago
> I suspect OP writes great, "bug-free" code in the backend / systems domain, but I don't think their testing practice or anecdotes are applicable across the industry. I'll grant OP a Magic Cloak of Error Abolishment and an enchanted ring with +4 INT, +4 WIS, and +3 VARIABLE-NAMING... Regardless, what's the very first thing I'm gonna do when I inherit his project for support or further development? Start ripping it apart to add tests so that I can verify the behaviour of the system pre, and post, change. If that code base was tested before, even poorly, that means extending the tests and improving their web of assertions and I will be making positive changes to the code within hours. If that code base is untested then my maintenance activities will involve unsupported refactoring and WAGs about system behaviour once the new tests come online. Days of work will hamper the start of maintenance activities. From experience, it will also likley mean finding a bunch of previously hidden issues, bugs, and suspect behaviour that has to be re-analyzed and re-tested before maintenance work can begin in earnest. A strong feeling that something is "bug-free" does not tell me that everything works unchanged on a new platform/OS/architecture, or let me throw in a newly conceived corner case, or check some environmental oddity... If we start out with the premise that systems spend most of their lifetime in maintenance, preparing the system for maintenance activities and trying to eliminate risk in that delicate window make a lot of sense. Not for a lone developer who has a bunch of domain knowledge in their heads, but for teams where new individuals are exposed to new domains and code at the same time.

loup-vaillant 9y ago

The FOR macro is more readable than the original C for loops. Given their sheer number in this code, I think this is worth the 10 seconds learning curve. I don't use that when I work with others, though. The "used twice" macro makes clear this is the exact same code. Would be hard to make sure it is otherwise. Easier to review that way (carry code is a nightmare to test and review). The constant time comparison is necessary to ensure timing attacks cannot happen. The arithmetic trick may be a tad slower than a conditional branch, but its timings are consistent. Note: the compiler is still allowed to just use a branch, but in practice they don't.