[Cryptech Tech] Doc, Markdown, and Wiki

[Bernd Paysan]

Am Mittwoch, 29. Oktober 2014, 08:49:17 schrieb Randy Bush:
> > http://wiki.cryptech.is/wiki/GitRepositories
> 
> ok.  it's cool.  very cool.

I've just audited the code I originally contributed, and I have a serious 
question, regarding how much fluff went into that code (it's a complete 
rewrite, and even leaving away the copyrigth message and the comments, it's 
about a three-fold expansion):

I do understand different people have different style, and my style is 
deliberately terse and short.  This is for security and audit reasons: The 
more you write code, the more bugs you make (it's pretty much a constant per 
LoC), and the more bugs will not be noticed when reading the code. Fluffing up 
code is something I do not aprove.

I went through several thousand lines of code written in that sort of style, 
and it's just something that adds up: looks harmless when applied to short 
code like the one I wrote, but in the end, you simply have three times as much 
as code to look through, and after an hour of audit, every auditor is 
exhausted, and stops paying attention.

If you want to go for good auditible code, do two things:

a) be terse
b) be consistent

The consistent style helps (only one style to learn), and the terseness helps.

I've looked through other code, and there, the fluffing up contains constructs 
which I would not let through, e.g. in chacha.v.  There, each register is 
writable by an address, and there is an async process to generate the write 
enables from the address, and a sync process where the write enables are 
actually checked.  This sort of stuff is tearing the logic apart into two 
blocks, which are separated by hundreds of lines of code in between, and this 
is completely unnecessary: Any decent synthesis tool can do that job just fine 
(maybe even better).  Don't do it this way.  Write the assignments directly in 
a case statement.  It takes out the entire async process for writing (not for 
reading), because it's not actually doing anything of value: It just adds 
lines and lines of code, can possibly introduce bugs and makes the live of an 
auditor harder, because he has to check if all those connections are done 
correctly.

The other thing about fluff is that you need to scroll to get through, and it 
just looks so regular that you won't check on everything.  When you read with 
a case statement, just do it that way:

    ADDR_KEY0: tmp_read_data = key0_reg;
    ADDR_KEY1: tmp_read_data = key1_reg;
    ADDR_KEY2: tmp_read_data = key2_reg;
    ADDR_KEY3: tmp_read_data = key3_reg;
    ADDR_KEY4: tmp_read_data = key4_reg;
    ADDR_KEY5: tmp_read_data = key5_reg;
    ADDR_KEY6: tmp_read_data = key6_reg;
    ADDR_KEY7: tmp_read_data = key7_reg;
...

It still has the same regularity, but now, you can check quite easily that the 
numbers on the left are a fit to the numbers on the right, because your eye 
doesn't have to move that far.

There's a way to do even better: Just declare the keys and the data as one 
single register, and use the corresponding part of the address to calculate 
the index.  That way, you compress repetitive code (humans are *bad* at 
getting repetitive tasks right, and it is boring for the auditor) into non-
repetitive code that has algorithmic parts, which is easy to verify once and 
be done with it.

parameter ADDR_KEYS        = 8'b00010zzz;

   casez(address)
      ADDR_KEYS:  tmp_read_data = key_reg[32*address[2:0]+31:32*address[2:0]];
...

Instead of using 32*address, you could also use
{address[2:0], 5'h1f}:{address[2:0], 5'h00}, if you wan to avoid the idea of 
"multiplying and adding in a register selection is expensive or looks like 
software".

That way, you collapse 5*24=120 lines into two (2) lines (the parameters and 
declarations not counted, where another 48 lines get replaced by 4 lines).  Do 
you understand how much more auditable 2 lines are than 120 lines?  This sort 
of fluffing up code in the chacha block is way worse than in the rosc block.

Yes, each line that way is harder to read than the easy repetitive lines, but 
the easy lines make it easy to hide something in them, and make the reviewer 
tired.  I don't want to imply that Joachim is going to hide something there, 
at least not intentionally, but there is an obvious connection between the 
address map and the idea this code expresses explicitly, which is only 
implicit in Joachim's code.

TL;DR

Short code is way better for auditability.  Compress code that contains an 
inherent calculated logic by actually writing that logic into the code.

-- 
Bernd Paysan
"If you want it done right, you have to do it yourself"
http://bernd-paysan.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.cryptech.is/archives/tech/attachments/20141029/81007bbd/attachment.sig>