[wip] Return a frozen array of bytes when producing keypairs

[steveluscher]

Preamble

• This is not a PR about cryptography, it's a PR about JavaScript/Typescript.
• I have no intention of landing this; I just want to start a discussion about API and type system design.

Discussion

Overspecification

It's a common thing to see an API overspecified. This means that its input types overstate what the implementation needs to function. APIs like nacl.sign.detached, for instance, ask that you bring a Uint8Array when really all the implementation requires is:

• Something that can be indexed into (ie. thing[1])
• Something that has a length property that is a number

tweetnacl doesn't need any of the other features offered by the Uint8Array interface, like .every() or .byteLength or any of that. The only other thing offered by that type is a guarantee that each element is a number between 0–255, but that can be either ignored (signatures will be junk otherwise) or validated at runtime.

Frozen objects

Uint8Array can not be frozen by Object.freeze which is useful to prevent runtime mutation. Now that the inputs to nacl.sign.detached are specified just enough, we can actually return plain arrays that can be frozen. This is a runtime protection against the values being mutated.

Opaque/branded types

Next, we can apply a type system level protection against the values being mutated.

declare const tag: unique symbol;
type SecretKey = ReadonlyArray<number> & { readonly [tag]: 'SecretKey' }

The idea here is that the only value for secretKey that can be supplied to nacl.sign.detached was something that was produced by nacl.sign.keyPair.*. If you try to pass an array that you constructed outside of nacl.sign.keyPair, it will fail to typecheck.

Also, if you try to mutate the value that you received from nacl.sign.keyPair, it will fail to typecheck.

Benchmarks

BEFORE

 % yarn bench                                                                                                                                                                                                                                                       Programming/tweetnacl-js (6a9594a) MacBook-Pro-3
yarn run v1.22.15
$ node test/benchmark/bench.js
crypto_stream_xor 1K                 21956 ops           0.02 ms/op     43864.75 ops/sec     42.84 MiB/s
crypto_onetimeauth 1K                11556 ops           0.04 ms/op     23054.74 ops/sec     22.51 MiB/s
crypto_secretbox 1K                   7857 ops           0.06 ms/op     15663.83 ops/sec     15.30 MiB/s
crypto_hash 1K                        4737 ops           0.11 ms/op      9446.69 ops/sec      9.23 MiB/s
crypto_hash 16K                        305 ops           1.64 ms/op       609.83 ops/sec      9.53 MiB/s
secretbox 1K                          7457 ops           0.07 ms/op     14902.83 ops/sec     14.55 MiB/s
secretbox.open 1K                     7001 ops           0.07 ms/op     13996.64 ops/sec     13.67 MiB/s
crypto_scalarmult_base                 137 ops           3.65 ms/op       273.85 ops/sec                
box 1K                                 129 ops           3.90 ms/op       256.34 ops/sec      0.25 MiB/s
box.open 1K                            133 ops           3.77 ms/op       265.56 ops/sec      0.26 MiB/s
sign                                    63 ops           8.03 ms/op       124.61 ops/sec                
sign.open                               31 ops          16.43 ms/op        60.86 ops/sec                
✨  Done in 6.25s.

AFTER

 % yarn bench                                                                                                                                                                                                                                    Programming/tweetnacl-js (frozen-keypair-return-type) MacBook-Pro-3
yarn run v1.22.15
$ node test/benchmark/bench.js
crypto_stream_xor 1K                 21892 ops           0.02 ms/op     43767.93 ops/sec     42.74 MiB/s
crypto_onetimeauth 1K                11428 ops           0.04 ms/op     22839.80 ops/sec     22.30 MiB/s
crypto_secretbox 1K                   7889 ops           0.06 ms/op     15744.50 ops/sec     15.38 MiB/s
crypto_hash 1K                        4771 ops           0.11 ms/op      9520.65 ops/sec      9.30 MiB/s
crypto_hash 16K                        306 ops           1.64 ms/op       610.96 ops/sec      9.55 MiB/s
secretbox 1K                          7345 ops           0.07 ms/op     14684.75 ops/sec     14.34 MiB/s
secretbox.open 1K                     7065 ops           0.07 ms/op     14102.68 ops/sec     13.77 MiB/s
crypto_scalarmult_base                 135 ops           3.72 ms/op       268.66 ops/sec                
box 1K                                 134 ops           3.75 ms/op       266.84 ops/sec      0.26 MiB/s
box.open 1K                            135 ops           3.71 ms/op       269.73 ops/sec      0.26 MiB/s
sign                                    60 ops           8.44 ms/op       118.44 ops/sec                
sign.open                               31 ops          16.41 ms/op        60.94 ops/sec                
✨  Done in 6.26s.

Related to #247.

[paulmillr]

Important: It only partially addresses it, many cases are still left w/o solution.

Quite a few libraries re-calculate public keys. There is no issue in doing that, instead of half-solutions.

[CMEONE]

Hey @steveluscher, nice to see a fellow Solana developer here! I guess this repo has been getting lots of eyes on it lately after the whole Slope wallet private key hack. It definitely seems like freezing these byte arrays is a smart choice. I'll definitely take a look at this code when I get back to my laptop in a few hours!