get mutable value: onreply rely on salt, but its not defined in bep

[mh-cbon]

hi,

in DHT.get method https://github.com/webtorrent/bittorrent-dht/blob/master/client.js#L223 when handling the reply, it calls for encodesigdata https://github.com/webtorrent/bittorrent-dht/blob/master/client.js#L261, defined at https://github.com/webtorrent/bittorrent-dht/blob/master/client.js#L649

ensodeSigData rely on the presence of the salt value in the reply packet to reproduce the bencoded value, but the bep44 does not define the salt field in the mutable get reply,

{
    "r":
    {
        "id": <20 byte id of sending node (string)>,
        "k": <ed25519 public key (32 bytes string)>,
        "nodes": <IPv4 nodes close to 'target'>,
        "nodes6": <IPv6 nodes close to 'target'>,
        "seq": <monotonically increasing sequence number (integer)>,
        "sig": <ed25519 signature (64 bytes string)>,
        "token": <write-token (string)>,
        "v": <any bencoded type, whose encoded size <= 1000>
    },
    "t": <transaction-id (string)>,
    "y": "r",
}

the salt value in this case must be provided by the person who initiates the get query, with the pbkey.

I had to change a little bit the module, such as,

setTimeout(function(){
  dht.get("3212924042a669733a6204b0f087797d72f715d6", {verify:ed.verify, salt:"foobar2"}, function(err, res){
    console.log("got",err)
    console.log("got",res)
  })
}, 2500)

In the onreply of get method, if (!verify(r.sig, encodeSigDataSalt(r, opts.salt), r.k)) return true

finally added,

function encodeSigDataSalt (msg, salt) {
  var ref = { seq: msg.seq || 0, v: msg.v }
  if (salt) ref.salt = salt
  return bencode.encode(ref).slice(1, -1)
}

I also changed the hash test equality to if (equals(hash(opts.salt ? Buffer.concat([r.k, Buffer.from(opts.salt, 'utf-8')]) : r.k), key)) { because salt is string.

BTW, can we improve the doc for mutable message usage with the module ?

thanks.

---

Note that the salt is not returned in the response to a get request. This is intentional. When issuing a get request for an item is expected to know what the salt is (because it is part of what the target ID that is being looked up is derived from). There is no need to repeat it back for bystanders to see.

When looking up a mutable item, the target field MUST be the SHA-1 hash of this key concatenated with the salt, if present.

So yes there are more changes to apply because in case of mutable items, the target is derived from those two data: pbk + salt
the current implementation assume it receives the target directly.

[chr15m]

Code to replicate and demonstrate this bug in action: https://github.com/chr15m/bittorrent-dht-salt-bug

[mh-cbon]

thanks @chr15m