Use constant time string comparison for checking key

[scriby]

https://github.com/jed/keygrip/blob/master/index.js#L38

That line uses a short circuiting comparison, which reveals timing information about the key used to an attacker. See http://codahale.com/a-lesson-in-timing-attacks/ for more details.

Here's a constant time comparison function you can use:

var constantTimeCompare = function(val1, val2){
    if(val1 == null && val2 != null){
        return false;
    } else if(val2 == null && val1 != null){
        return false;
    } else if(val1 == null && val2 == null){
        return true;
    }

    if(val1.length !== val2.length){
        return false;
    }

    var matches = 1;

    for(var i = 0; i < val1.length; i++){
        matches &= (val1.charAt(i) === val2.charAt(i) ? 1 : 0); //Don't short circuit
    }

    return matches === 1;
};

[jed]

thanks, @scriby. do you think it would be sufficient to keep it light by .maping the signatures and then doing an .indexOf instead?

[scriby]

Can you show a code example of what you're thinking?

[jed]

this.index = function(data, digest) {
  var signKey = sign.bind(null, data)

  return keys.map(signKey).indexOf(digest)
}

[scriby]

Ah, I don't think that solves the problem. The problem is in comparing the signature strings themselves, so the indexOf will have the same issue when comparing the digest to the array elements.

I think you just have to replace

if (digest === sign(data, keys[i])) return i

with

if(constantTimeCompare(digest, sign(data, keys[i])) return i

[jed]

sounds good. would you mind teeing up a pull request?