make PrivateKey + Signature comparisons use constant time comparisons

[odeke-em]

Fixes #43

Avoid susceptibility to timing/side channel attacks by ensuring
that private key and signature comparisons use
subtle.ConstantTimeCompare
instead of
bytes.Equal

[ebuchman]

Thanks @odeke-em . Seems like a good idea.

But does it matter for signatures? And where do we even use privKey.Equals ?

[odeke-em]

@ebuchman, yeah for signatures I infer that from the case that those are like authenticated messages, but I perhaps am wrong that it doesn't matter.

In regards to where we use PrivKey.Equals, interestingly I don't seem to see usages in our repos.
If the usage/correctness doesn't matter, then perhaps we should delete those otherwise it is something that will get the ears of any auditor burning. I found this by isolating package by package and reading through to ensure just sort of correctness of the code.

[ebuchman]

Hm, seems maybe we can eliminate both Equals ? I see no reason to ever be checking privkey equality like that, and same for signatures? Are we using that anywhere?

[zramsay]

status?

[ebuchman]

Replaced by https://github.com/tendermint/go-crypto/pull/53/files

[ebuchman]

@odeke-em note subtle uses 1 to denote equality: https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare

Also, is there some reason the signatures need to be checked in constant time?

[odeke-em]

Thank you @ebuchman for the refurbish! Mother of God, why did I use 0 instead? Moreover I was somewhat involved in reviewing the original Go package signature IIRC, how could I not catch that.

[ebuchman]

oh cool - agl doesn't think verify should be in constant time either: golang/crypto@c412588#diff-8183aa2c95dc1385c8f58bf1917fcb0b

[odeke-em]

Gotcha gotcha, thanks for the discussion and you called this about signatures not needing constant time comparisons.