v1 shards pass v2 validateShard function

[m4gpi]

Expected Behavior

When passing a version 1 shard, such as 80104424cf070d06548da4dd40d47f4c156, the validateShard function should return false when it's told its dealing with a version 2 shard.

Current Behavior

A v1 shard currently passes validation. This is because the secrets.js-grempe doesn't verify the data component it is passed. If passed the shard as above:

const secrets = require('secrets.js-grempe')
secrets.extractShareComponents(shard)
// returns {
//   bits: 8,
//   id: 1,
//   data: 'd38e36e1c7f4ef4774eb9e3c75ae1d778d1de3b7f8735e7a'
// }

Currently we have no handler to deal with this.

Steps to Reproduce

const darkCrystal = require('dark-crystal-secrets')
const shard = '80104424cf070d06548da4dd40d47f4c156' // a v1 shard
darkCrystal.validateShard(shard, '2.0.0')
// returns true - should really return false as its not actually a valid shard in this context

Context

This actually has a knock on effect that could be quite problematic. If a v1 shard is returned / forwarded to someone requesting v2 shards, it will cause the secret to fail to reconstruct. As far as the algorithm is concerned, the MAC for this shard isn't the same, and it flags the error:

Error: This secret appears to be corrupt - invalid MAC
    at hex2Secret (/home/kyphae/projects/dark-crystal-secrets/v2.js:54:34)
    at Object.combineV2 [as combine] (/home/kyphae/projects/dark-crystal-secrets/v2.js:29:12)
    at Object.combine (/home/kyphae/projects/dark-crystal-secrets/index.js:16:31)
    at exports.combine (/home/kyphae/projects/dark-crystal-secrets-api/app/controllers/secrets-controller.js:40:32)
    at Layer.handle [as handle_request] (/home/kyphae/projects/dark-crystal-secrets-api/node_modules/express/lib/router/layer.js:95:5)
    at next (/home/kyphae/projects/dark-crystal-secrets-api/node_modules/express/lib/router/route.js:137:13)
    at runner.then.errors (/home/kyphae/projects/dark-crystal-secrets-api/node_modules/express-validator/check/check.js:16:7)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

It could be the case that the person actually has all the relevant parts already, but this one rogue one will cause reconstruction to fail.

Ideas / Solutions

• At least when reconstructing the secret, we can check to see if all the shards are the same length, and omit the ones that are not in the majority. However, this doesn't solve the problem whereby an attacker spams the recipient's data with dodgy shards.
• Completely deprecate v1 shards and ensure any future iterations adhere to the standard we set in v2 as a minimum.