From eaf4b8c79500e7be9b227c5dca8b9813bb75842d Mon Sep 17 00:00:00 2001
From: Ethan Buchman <ethan@coinculture.info>
Date: Thu, 21 Dec 2017 19:51:57 -0500
Subject: [PATCH] fix Equals

---
 priv_key.go       | 16 ++++++----------
 signature.go      | 16 ++++------------
 signature_test.go | 24 ++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/priv_key.go b/priv_key.go
index 456985615..613d94593 100644
--- a/priv_key.go
+++ b/priv_key.go
@@ -55,13 +55,11 @@ func (privKey PrivKeyEd25519) PubKey() PubKey {
 	return PubKeyEd25519(pubBytes).Wrap()
 }
 
+// Equals - you probably don't need to use this.
+// Runs in constant time based on length of the keys.
 func (privKey PrivKeyEd25519) Equals(other PrivKey) bool {
 	if otherEd, ok := other.Unwrap().(PrivKeyEd25519); ok {
-		// It is essential that we constant time compare
-		// private keys and signatures instead of bytes.Equal,
-		// to avoid susceptibility to timing/side channel attacks.
-		// See Issue https://github.com/tendermint/go-crypto/issues/43
-		return subtle.ConstantTimeCompare(privKey[:], otherEd[:]) == 0
+		return subtle.ConstantTimeCompare(privKey[:], otherEd[:]) == 1
 	} else {
 		return false
 	}
@@ -146,13 +144,11 @@ func (privKey PrivKeySecp256k1) PubKey() PubKey {
 	return pub.Wrap()
 }
 
+// Equals - you probably don't need to use this.
+// Runs in constant time based on length of the keys.
 func (privKey PrivKeySecp256k1) Equals(other PrivKey) bool {
 	if otherSecp, ok := other.Unwrap().(PrivKeySecp256k1); ok {
-		// It is essential that we constant time compare
-		// private keys and signatures instead of bytes.Equal,
-		// to avoid susceptibility to timing/side channel attacks.
-		// See Issue https://github.com/tendermint/go-crypto/issues/43
-		return subtle.ConstantTimeCompare(privKey[:], otherSecp[:]) == 0
+		return subtle.ConstantTimeCompare(privKey[:], otherSecp[:]) == 1
 	} else {
 		return false
 	}
diff --git a/signature.go b/signature.go
index fb07aba96..be1f24905 100644
--- a/signature.go
+++ b/signature.go
@@ -1,7 +1,7 @@
 package crypto
 
 import (
-	"crypto/subtle"
+	"bytes"
 	"fmt"
 
 	"github.com/tendermint/go-wire"
@@ -46,11 +46,7 @@ func (sig SignatureEd25519) String() string { return fmt.Sprintf("/%X.../", Fing
 
 func (sig SignatureEd25519) Equals(other Signature) bool {
 	if otherEd, ok := other.Unwrap().(SignatureEd25519); ok {
-		// It is essential that we constant time compare
-		// private keys and signatures instead of bytes.Equal,
-		// to avoid susceptibility to timing/side channel attacks.
-		// See Issue https://github.com/tendermint/go-crypto/issues/43
-		return subtle.ConstantTimeCompare(sig[:], otherEd[:]) == 0
+		return bytes.Equal(sig[:], otherEd[:])
 	} else {
 		return false
 	}
@@ -85,12 +81,8 @@ func (sig SignatureSecp256k1) IsZero() bool { return len(sig) == 0 }
 func (sig SignatureSecp256k1) String() string { return fmt.Sprintf("/%X.../", Fingerprint(sig[:])) }
 
 func (sig SignatureSecp256k1) Equals(other Signature) bool {
-	if otherEd, ok := other.Unwrap().(SignatureSecp256k1); ok {
-		// It is essential that we constant time compare
-		// private keys and signatures instead of bytes.Equal,
-		// to avoid susceptibility to timing/side channel attacks.
-		// See Issue https://github.com/tendermint/go-crypto/issues/43
-		return subtle.ConstantTimeCompare(sig[:], otherEd[:]) == 0
+	if otherSecp, ok := other.Unwrap().(SignatureSecp256k1); ok {
+		return bytes.Equal(sig[:], otherSecp[:])
 	} else {
 		return false
 	}
diff --git a/signature_test.go b/signature_test.go
index 5e9f06723..4801e5fef 100644
--- a/signature_test.go
+++ b/signature_test.go
@@ -141,3 +141,27 @@ func TestWrapping(t *testing.T) {
 	}
 
 }
+
+func TestPrivKeyEquality(t *testing.T) {
+	{
+		privKey := GenPrivKeySecp256k1().Wrap()
+		privKey2 := GenPrivKeySecp256k1().Wrap()
+		assert.False(t, privKey.Equals(privKey2))
+		assert.False(t, privKey2.Equals(privKey))
+
+		privKeyCopy := privKey // copy
+		assert.True(t, privKey.Equals(privKeyCopy))
+		assert.True(t, privKeyCopy.Equals(privKey))
+	}
+
+	{
+		privKey := GenPrivKeyEd25519().Wrap()
+		privKey2 := GenPrivKeyEd25519().Wrap()
+		assert.False(t, privKey.Equals(privKey2))
+		assert.False(t, privKey2.Equals(privKey))
+
+		privKeyCopy := privKey // copy
+		assert.True(t, privKey.Equals(privKeyCopy))
+		assert.True(t, privKeyCopy.Equals(privKey))
+	}
+}