CheckBadBinding when registration

client/coniksclient/internal/cmd/register.go

@@ -70,9 +70,20 @@ Example call:
 				// TODO: Save the cc to verify the TB and for later
 				// usage (TOFU checks)
 			case p.ReqNameExisted:
-				// TODO: See #133
 				fmt.Println("Name is already registered.")
 			}
+		case p.CheckBindingsDiffer:
+			switch response.Error {
+			case p.ReqNameExisted:
+				fmt.Println(`Are you trying to update your binding? Unfortunately, KeyChange isn't supported yet.`)
+			case p.ReqSuccess:
+				fmt.Println("Oops! The server snuck in some other key.")
+				recvKey, err := response.GetKey()
+				if err != nil {
+					fmt.Println("Cannot get the key from the response, error: " + err.Error())
+				}
+				fmt.Println("[" + string(recvKey) + "] was registered instead of [" + string(key) + "]")
+			}
 		default:
 			fmt.Println("Error: " + err.Error())
 		}

merkletree/proof.go

@@ -2,11 +2,26 @@ package merkletree
 
 import (
 	"bytes"
+	"errors"
 
 	"github.com/coniks-sys/coniks-go/crypto"
 	"github.com/coniks-sys/coniks-go/utils"
 )
 
+var (
+	// ErrBindingsDiffer indicates that the value included in the proof
+	// is different from the expected value.
+	ErrBindingsDiffer = errors.New("[merkletree] The values included in the bindings are different")
+	// ErrUnverifiableCommitment indicates that the leaf node's commitment is unverifiable.
+	ErrUnverifiableCommitment = errors.New("[merkletree] Could not verify the commitment")
+	// ErrIndicesMismatch indicates that there is a mismatch
+	// between the lookup index and the leaf index.
+	ErrIndicesMismatch = errors.New("[merkletree] The lookup index is inconsistent with the index of the proof node")
+	// ErrUnequalTreeHashes indicates that the hash computed from the authentication path
+	// and the hash taken from the signed tree root are different.
+	ErrUnequalTreeHashes = errors.New("[merkletree] The hashes computed from the authentication path and the STR are unequal")
+)
+
 // ProofNode can be a user node or an empty node,
 // which is included in the returned AuthenticationPath
 // of a given index. The type of that node can be determined
@@ -80,38 +95,45 @@ func (ap *AuthenticationPath) authPathHash() []byte {
 	return hash
 }
 
-func (ap *AuthenticationPath) verifyBinding(key, value []byte) bool {
-	return bytes.Equal(ap.Leaf.Value, value) &&
-		ap.Leaf.Commitment.Verify(key, value)
-}
-
-// Verify recomputes the tree's root node from the authentication path,
+// Verify first compares the lookup index with the leaf index.
+// It expects the lookup index and the leaf index match in the
+// first l bits with l is the Level of the proof node if ap is
+// a proof of absence. It also verifies the value and
+// the commitment (in case of the proof of inclusion).
+// Finally, it recomputes the tree's root node from ap,
 // and compares it to treeHash, which is taken from a STR.
-// Specifically, treeHash has to come from the STR whose tree returns
-// the authentication path.
+// Specifically, treeHash has to come from the STR whose tree returns ap.
+//
 // This should be called after the VRF index is verified successfully.
-func (ap *AuthenticationPath) Verify(key, value, treeHash []byte) bool {
-	if ap.ProofType() == ProofOfAbsence { // proof of absence
+func (ap *AuthenticationPath) Verify(key, value, treeHash []byte) error {
+	if ap.ProofType() == ProofOfAbsence {
 		// Check if i and j match in the first l bits
 		indexBits := utils.ToBits(ap.Leaf.Index)
 		lookupIndexBits := utils.ToBits(ap.LookupIndex)
 		for i := 0; i < int(ap.Leaf.Level); i++ {
 			if indexBits[i] != lookupIndexBits[i] {
-				return false
+				return ErrIndicesMismatch
 			}
 		}
-	} else { // proof of inclusion
+		// expect the value is nil since we suppressed
+		// the salt & value (see Get())
+		if ap.Leaf.Value != nil {
+			return ErrBindingsDiffer
+		}
+	} else {
 		// Verify the key-value binding returned in the ProofNode
-		if !ap.verifyBinding(key, value) {
-			return false
+		if !bytes.Equal(ap.Leaf.Value, value) {
+			return ErrBindingsDiffer
+		}
+		if !ap.Leaf.Commitment.Verify(key, value) {
+			return ErrUnverifiableCommitment
 		}
 	}
 
-	// step 2. Verify the auth path of the returned leaf
 	if !bytes.Equal(treeHash, ap.authPathHash()) {
-		return false
+		return ErrUnequalTreeHashes
 	}
-	return true
+	return nil
 }
 
 func (ap *AuthenticationPath) ProofType() ProofType {

merkletree/proof_test.go

@@ -63,7 +63,7 @@ func TestVerifyProof(t *testing.T) {
 		t.Fatal("Expect a proof of inclusion")
 	}
 	// verify auth path
-	if !proof.Verify([]byte(key3), val3, m.hash) {
+	if proof.Verify([]byte(key3), val3, m.hash) != nil {
 		t.Error("Proof of inclusion verification failed.")
 	}
 
@@ -78,7 +78,7 @@ func TestVerifyProof(t *testing.T) {
 		!bytes.Equal(vrfPrivKey1.Compute([]byte("123")), proof.LookupIndex) {
 		t.Fatal("Expect a proof of absence")
 	}
-	if !proof.Verify([]byte("123"), nil, m.hash) {
+	if proof.Verify([]byte("123"), nil, m.hash) != nil {
 		t.Error("Proof of absence verification failed.")
 	}
 }
@@ -111,7 +111,7 @@ func TestVerifyProofSamePrefix(t *testing.T) {
 		utils.ToBytes(utils.ToBits(absentIndex)[:proof.Leaf.Level])) {
 		t.Fatal("Expect these indices share the same prefix in the first bit")
 	}
-	if !proof.Verify([]byte("a"), nil, m.hash) {
+	if proof.Verify([]byte("a"), nil, m.hash) != nil {
 		t.Error("Proof of absence verification failed.")
 	}
 
@@ -128,7 +128,66 @@ func TestVerifyProofSamePrefix(t *testing.T) {
 		t.Fatal("Expect a proof of inclusion")
 	}
 	// step 2. verify auth path
-	if !proof.Verify([]byte(key1), val1, m.hash) {
+	if proof.Verify([]byte(key1), val1, m.hash) != nil {
 		t.Error("Proof of inclusion verification failed.")
 	}
 }
+
+func TestProofVerificationErrors(t *testing.T) {
+	m, err := NewMerkleTree()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	key1 := "key1"
+	index1 := vrfPrivKey1.Compute([]byte(key1))
+	val1 := []byte("value1")
+	if err := m.Set(index1, key1, val1); err != nil {
+		t.Fatal(err)
+	}
+	m.recomputeHash()
+	absentIndex := vrfPrivKey1.Compute([]byte("a"))
+
+	// ProofOfInclusion
+	// assert proof of inclusion
+	proof1 := m.Get(index1)
+	if proof1.ProofType() != ProofOfInclusion {
+		t.Fatal("Expect a proof of inclusion")
+	}
+	// - ErrBindingsDiffer
+	proof1.Leaf.Value[0] += 1
+	if err := proof1.Verify([]byte(key1), val1, m.hash); err != ErrBindingsDiffer {
+		t.Error("Expect", ErrBindingsDiffer, "got", err)
+	}
+	// - ErrUnverifiableCommitment
+	proof1.Leaf.Value[0] -= 1
+	proof1.Leaf.Commitment.Salt[0] += 1
+	if err := proof1.Verify([]byte(key1), val1, m.hash); err != ErrUnverifiableCommitment {
+		t.Error("Expect", ErrUnverifiableCommitment, "got", err)
+	}
+	// ErrUnequalTreeHashes
+	hash := append([]byte{}, m.hash...)
+	hash[0] += 1
+	proof1.Leaf.Commitment.Salt[0] -= 1
+	if err := proof1.Verify([]byte(key1), val1, hash); err != ErrUnequalTreeHashes {
+		t.Error("Expect", ErrUnequalTreeHashes, "got", err)
+	}
+
+	// ProofOfAbsence
+	proof2 := m.Get(absentIndex) // shares the same prefix with leaf node key1
+	// assert proof of absence
+	if proof2.ProofType() != ProofOfAbsence {
+		t.Fatal("Expect a proof of absence")
+	}
+	// - ErrBindingsDiffer
+	proof2.Leaf.Value = make([]byte, 1)
+	if err := proof2.Verify([]byte("a"), nil, m.hash); err != ErrBindingsDiffer {
+		t.Error("Expect", ErrBindingsDiffer, "got", err)
+	}
+	// - ErrIndicesMismatch
+	proof2.Leaf.Value = nil
+	proof2.Leaf.Index[0] &= 0x01
+	if err := proof2.Verify([]byte("a"), nil, m.hash); err != ErrIndicesMismatch {
+		t.Error("Expect", ErrIndicesMismatch, "got", err)
+	}
+}

protocol/consistencychecks.go

@@ -224,12 +224,20 @@ func verifyAuthPath(uname string, key []byte,
 		key = ap.Leaf.Value
 	}
 
-	// verify auth path
-	if !ap.Verify([]byte(uname), key, str.TreeHash) {
+	switch err := ap.Verify([]byte(uname), key, str.TreeHash); err {
+	case m.ErrBindingsDiffer:
+		return CheckBindingsDiffer
+	case m.ErrUnverifiableCommitment:
+		return CheckBadCommitment
+	case m.ErrIndicesMismatch:
+		return CheckBadLookupIndex
+	case m.ErrUnequalTreeHashes:
 		return CheckBadAuthPath
+	case nil:
+		return nil
+	default:
+		panic("[coniks] Unknown error: " + err.Error())
 	}
-
-	return nil
 }
 
 func (cc *ConsistencyChecks) updateTBs(requestType int, msg *Response,
@@ -313,14 +321,14 @@ func (cc *ConsistencyChecks) verifyReturnedPromise(df *DirectoryProof,
 		return CheckBadSignature
 	}
 
-	// key could be nil if we have no information about
-	// the existed binding (TOFU).
-	if key == nil {
-		key = tb.Value
+	if !bytes.Equal(tb.Index, ap.LookupIndex) {
+		return CheckBadPromise
 	}
 
-	if tb.Verify(ap.LookupIndex, key) {
-		return nil
+	// key could be nil if we have no information about
+	// the existed binding (TOFU).
+	if key != nil && !bytes.Equal(tb.Value, key) {
+		return CheckBindingsDiffer
 	}
-	return CheckBadPromise
+	return nil
 }

protocol/consistencychecks_test.go

@@ -83,8 +83,7 @@ func TestVerifyRegistrationResponseWithTB(t *testing.T) {
 	}
 
 	// test error name existed with different key
-	// FIXME: see #133. It should return something like CheckBadBinding.
-	if e1, e2 := registerAndVerify(d, cc, alice, []byte{1, 2, 3}); e1 != ReqNameExisted || e2 != CheckBadPromise {
+	if e1, e2 := registerAndVerify(d, cc, alice, []byte{1, 2, 3}); e1 != ReqNameExisted || e2 != CheckBindingsDiffer {
 		t.Error(e1)
 		t.Error(e2)
 	}
@@ -101,6 +100,10 @@ func TestVerifyRegistrationResponseWithTB(t *testing.T) {
 		t.Error(e1)
 		t.Error(e2)
 	}
+	if e1, e2 := registerAndVerify(d, cc, alice, []byte{1, 2, 3}); e1 != ReqNameExisted || e2 != CheckBindingsDiffer {
+		t.Error(e1)
+		t.Error(e2)
+	}
 }
 
 func TestVerifyFullfilledPromise(t *testing.T) {

protocol/error.go

@@ -29,6 +29,9 @@ const (
 	CheckPassed ErrorCode = iota + 200
 	CheckBadSignature
 	CheckBadVRFProof
+	CheckBindingsDiffer
+	CheckBadCommitment
+	CheckBadLookupIndex
 	CheckBadAuthPath
 	CheckBadSTR
 	CheckBadPromise
@@ -56,13 +59,16 @@ var (
 		ErrDirectory:                 "[coniks] Directory error",
 		ErrMalformedDirectoryMessage: "[coniks] Malformed directory message",
 
-		CheckPassed:        "[coniks] Consistency checks passed",
-		CheckBadSignature:  "[coniks] Directory's signature on STR or TB is invalid",
-		CheckBadVRFProof:   "[coniks] Returned index is not valid for the given name",
-		CheckBadAuthPath:   "[coniks] Returned binding is inconsistent with the tree root hash",
-		CheckBadSTR:        "[coniks] The hash chain is inconsistent",
-		CheckBadPromise:    "[coniks] The directory returned an invalid registration promise",
-		CheckBrokenPromise: "[coniks] The directory broke the registration promise",
+		CheckPassed:         "[coniks] Consistency checks passed",
+		CheckBadSignature:   "[coniks] Directory's signature on STR or TB is invalid",
+		CheckBadVRFProof:    "[coniks] Returned index is not valid for the given name",
+		CheckBindingsDiffer: "[coniks] The key in the binding is inconsistent with our expectation",
+		CheckBadCommitment:  "[coniks] The name-to-key binding commitment is not verifiable",
+		CheckBadLookupIndex: "[coniks] The lookup index is inconsistent with the index of the proof node",
+		CheckBadAuthPath:    "[coniks] Returned binding is inconsistent with the tree root hash",
+		CheckBadSTR:         "[coniks] The hash chain is inconsistent",
+		CheckBadPromise:     "[coniks] The directory returned an invalid registration promise",
+		CheckBrokenPromise:  "[coniks] The directory broke the registration promise",
 	}
 )
 

protocol/tb.go

@@ -2,10 +2,6 @@
 
 package protocol
 
-import (
-	"bytes"
-)
-
 // A TemporaryBinding consists of the private
 // Index for a username, the Value (i.e. public key etc.)
 // mapped to this index in a key directory, and a digital
@@ -33,11 +29,3 @@ func (tb *TemporaryBinding) Serialize(strSig []byte) []byte {
 	tbBytes = append(tbBytes, tb.Value...)
 	return tbBytes
 }
-
-// Verify validates the received tb by comparing
-// index, value against the Index and Value
-// of tb.
-func (tb *TemporaryBinding) Verify(index, value []byte) bool {
-	return bytes.Equal(tb.Index, index) &&
-		(value != nil && bytes.Equal(tb.Value, value))
-}