Key generation: clean up confusing / unused code and deprecation warning

[cinnamon-bun]

This is a small issue about confusing code and deprecation warnings. I don't think any functionality is broken.

https://github.com/ssbc/ssb-keys/blob/master/sodium.js#L9-L11

 generate: function (seed) {
    if(!seed) sodium.randombytes(seed = new Buffer(32))
    var keys = seed ? sodium.crypto_sign_seed_keypair(seed) : sodium.crypto_sign_keypair()

seed is the bytes to use as the private key. If omitted, we make random bytes.

Unusually, this code does an assignment inside a function call (seed = new Buffer...

So by the time we get to seed ? ..., seed is always defined, so we always execute the first half of the ternary if statement and never the second half.

(The second half of the if statement would have just generated a random secret key, same as we're doing manually here, so the outcome is the same)

Also new Buffer(32) prints a deprecation warning. The new way is Buffer.alloc(32). The potential security issue is that the buffer is not zeroed so it could contain program memory. But in this case we immediately fill it with random bytes, so it's fine.

So this code could be simpler and more clear than it is, maybe like:

generate: function (seed) {
    // seed should be undefined or a Buffer of 32 bytes to use as the private key
    if (!seed) {
        seed = Buffer.alloc(32)
        sodium.randombytes(seed)
    }
    var keys = sodium.crypto_sign_seed_keypair(seed)

[davegomez]

@cinnamon-bun Buffer.alloc(32) always return a buffer filled with 0s. I wonder how good is to use always the same value as seed.

On the other hand we could use Crypto.randomBytes(32) to create a "pseudo unique" seed to generate the keys.

What do you think?

[austinfrey]

edit: partially fixed in https://github.com/ssb-js/ssb-keys/pull/76

@davegomez your question gave me a moment of pause :) so I looked into it further. sodium.randomBytes(seed) does not use the seed buffer to generate the random bytes. it generates the random bytes and writes them to the seed buffer. So I think it's OK in this case to use a zero filled buffer initially.