Remove a redundant ErrChecksumMismatch error in tuf/client, since we

already have a ErrMismatchedChecksum in tuf/data/errors.

Also, have the CheckHashes function take a role name so that the
ErrMismatchedChecksum error can include the role name.

Signed-off-by: Ying Li <ying.li@docker.com>

client/client_update_test.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/docker/notary/passphrase"
 	"github.com/docker/notary/trustpinning"
-	"github.com/docker/notary/tuf/client"
 	"github.com/docker/notary/tuf/data"
 	"github.com/docker/notary/tuf/signed"
 	"github.com/docker/notary/tuf/store"
@@ -753,7 +752,7 @@ func testUpdateRemoteFileChecksumWrong(t *testing.T, opts updateOpts, errExpecte
 		if opts.role == data.CanonicalTimestampRole {
 			_, rightError = err.(store.ErrMaliciousServer)
 		} else {
-			_, isErrChecksum := err.(client.ErrChecksumMismatch)
+			_, isErrChecksum := err.(data.ErrMismatchedChecksum)
 			_, isErrMaliciousServer := err.(store.ErrMaliciousServer)
 			rightError = isErrChecksum || isErrMaliciousServer
 		}
@@ -924,7 +923,7 @@ func waysToMessUpServerNonRootPerRole(t *testing.T) map[string][]swizzleExpectat
 	}
 	perRoleSwizzling[data.CanonicalTargetsRole] = []swizzleExpectations{{
 		desc:       fmt.Sprintf("target missing delegations data"),
-		expectErrs: []interface{}{client.ErrChecksumMismatch{}},
+		expectErrs: []interface{}{data.ErrMismatchedChecksum{}},
 		swizzle: func(s *testutils.MetadataSwizzler, role string) error {
 			return s.MutateTargets(func(tg *data.Targets) {
 				tg.Delegations.Roles = tg.Delegations.Roles[1:]

cmd/notary/tuf.go

@@ -512,7 +512,7 @@ func (t *tufCommander) tufVerify(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("error retrieving target by name:%s, error:%v", targetName, err)
 	}
 
-	if err := data.CheckHashes(payload, target.Hashes); err != nil {
+	if err := data.CheckHashes(payload, targetName, target.Hashes); err != nil {
 		return fmt.Errorf("data not present in the trusted collection, %v", err)
 	}
 

server/timestamp/timestamp.go

@@ -124,7 +124,8 @@ func timestampExpired(ts *data.SignedTimestamp) bool {
 func snapshotExpired(ts *data.SignedTimestamp, snapshot []byte) bool {
 	// If this check failed, it means the current snapshot was not exactly what we expect
 	// via the timestamp. So we can consider it to be "expired."
-	return data.CheckHashes(snapshot, ts.Signed.Meta[data.CanonicalSnapshotRole].Hashes) != nil
+	return data.CheckHashes(snapshot, data.CanonicalSnapshotRole,
+		ts.Signed.Meta[data.CanonicalSnapshotRole].Hashes) != nil
 }
 
 // CreateTimestamp creates a new timestamp. If a prev timestamp is provided, it

tuf/client/client.go

@@ -98,7 +98,7 @@ func (c Client) checkRoot() error {
 		return err
 	}
 
-	if err := data.CheckHashes(raw, expectedHashes); err != nil {
+	if err := data.CheckHashes(raw, role, expectedHashes); err != nil {
 		return fmt.Errorf("Cached root hashes did not match snapshot root hashes")
 	}
 
@@ -160,7 +160,7 @@ func (c *Client) downloadRoot() error {
 		logrus.Debug("didn't find a cached root, must download")
 		download = true
 	} else {
-		if err := data.CheckHashes(cachedRoot, expectedHashes); err != nil {
+		if err := data.CheckHashes(cachedRoot, role, expectedHashes); err != nil {
 			logrus.Debug("cached root's hash didn't match expected, must download")
 			download = true
 		}
@@ -343,7 +343,7 @@ func (c *Client) downloadSnapshot() error {
 		download = true
 	} else {
 		// file may have been tampered with on disk. Always check the hash!
-		if err := data.CheckHashes(raw, expectedHashes); err != nil {
+		if err := data.CheckHashes(raw, role, expectedHashes); err != nil {
 			logrus.Debug("hash of snapshot in cache did not match expected hash, must download")
 			download = true
 		}
@@ -454,8 +454,8 @@ func (c *Client) downloadSigned(role string, size int64, expectedHashes data.Has
 	}
 
 	if expectedHashes != nil {
-		if err := data.CheckHashes(raw, expectedHashes); err != nil {
-			return nil, nil, ErrChecksumMismatch{role: role}
+		if err := data.CheckHashes(raw, role, expectedHashes); err != nil {
+			return nil, nil, data.ErrMismatchedChecksum{}
 		}
 	}
 
@@ -488,7 +488,7 @@ func (c Client) getTargetsFile(role string, snapshotMeta data.Files, consistent
 		download = true
 	} else {
 		// file may have been tampered with on disk. Always check the hash!
-		if err := data.CheckHashes(raw, expectedHashes); err != nil {
+		if err := data.CheckHashes(raw, role, expectedHashes); err != nil {
 			download = true
 		}
 

tuf/client/client_test.go

@@ -259,7 +259,7 @@ func TestChecksumMismatch(t *testing.T) {
 	remoteStorage.SetMeta("targets", orig)
 
 	_, _, err = client.downloadSigned("targets", int64(len(orig)), origHashes)
-	require.IsType(t, ErrChecksumMismatch{}, err)
+	require.IsType(t, data.ErrMismatchedChecksum{}, err)
 }
 
 func TestChecksumMatch(t *testing.T) {
@@ -300,7 +300,7 @@ func TestSizeMismatchLong(t *testing.T) {
 	_, _, err = client.downloadSigned("targets", l, origHashes)
 	// size just limits the data received, the error is caught
 	// either during checksum verification or during json deserialization
-	require.IsType(t, ErrChecksumMismatch{}, err)
+	require.IsType(t, data.ErrMismatchedChecksum{}, err)
 }
 
 func TestSizeMismatchShort(t *testing.T) {
@@ -322,7 +322,7 @@ func TestSizeMismatchShort(t *testing.T) {
 	_, _, err = client.downloadSigned("targets", l, origHashes)
 	// size just limits the data received, the error is caught
 	// either during checksum verification or during json deserialization
-	require.IsType(t, ErrChecksumMismatch{}, err)
+	require.IsType(t, data.ErrMismatchedChecksum{}, err)
 }
 
 func TestDownloadTargetsHappy(t *testing.T) {
@@ -500,7 +500,7 @@ func TestDownloadTargetChecksumMismatch(t *testing.T) {
 	repo.Snapshot = &snap
 
 	err = client.downloadTargets("targets")
-	require.IsType(t, ErrChecksumMismatch{}, err)
+	require.IsType(t, data.ErrMismatchedChecksum{}, err)
 }
 
 // TestDownloadTargetsNoChecksum: it's never valid to download any targets
@@ -615,7 +615,7 @@ func TestUpdateDownloadRootBadChecksum(t *testing.T) {
 	require.NoError(t, err)
 
 	err = client.downloadRoot()
-	require.IsType(t, ErrChecksumMismatch{}, err)
+	require.IsType(t, data.ErrMismatchedChecksum{}, err)
 }
 
 func TestUpdateDownloadRootChecksumNotFound(t *testing.T) {

tuf/client/errors.go

@@ -4,15 +4,6 @@ import (
 	"fmt"
 )
 
-// ErrChecksumMismatch - a checksum failed verification
-type ErrChecksumMismatch struct {
-	role string
-}
-
-func (e ErrChecksumMismatch) Error() string {
-	return fmt.Sprintf("tuf: checksum for %s did not match", e.role)
-}
-
 // ErrCorruptedCache - local data is incorrect
 type ErrCorruptedCache struct {
 	file string

tuf/data/errors.go

@@ -12,13 +12,14 @@ func (e ErrInvalidMetadata) Error() string {
 	return fmt.Sprintf("%s type metadata invalid: %s", e.role, e.msg)
 }
 
-// ErrMissingMeta - couldn't find the FileMeta object for a role or target
+// ErrMissingMeta - couldn't find the FileMeta object for the given Role, or
+// the FileMeta object contained no supported checksums
 type ErrMissingMeta struct {
 	Role string
 }
 
 func (e ErrMissingMeta) Error() string {
-	return fmt.Sprintf("tuf: sha256 checksum required for %s", e.Role)
+	return fmt.Sprintf("no checksums for supported algorithms were provided for %s", e.Role)
 }
 
 // ErrInvalidChecksum is the error to be returned when checksum is invalid
@@ -32,9 +33,12 @@ func (e ErrInvalidChecksum) Error() string {
 
 // ErrMismatchedChecksum is the error to be returned when checksum is mismatched
 type ErrMismatchedChecksum struct {
-	alg string
+	alg      string
+	name     string
+	expected string
 }
 
 func (e ErrMismatchedChecksum) Error() string {
-	return fmt.Sprintf("%s checksum mismatched", e.alg)
+	return fmt.Sprintf("%s checksum for %s did not match: expected %s", e.alg, e.name,
+		e.expected)
 }

tuf/data/snapshot.go

@@ -126,7 +126,9 @@ func (sp *SignedSnapshot) AddMeta(role string, meta FileMeta) {
 // not found
 func (sp *SignedSnapshot) GetMeta(role string) (*FileMeta, error) {
 	if meta, ok := sp.Signed.Meta[role]; ok {
-		return &meta, nil
+		if _, ok := meta.Hashes["sha256"]; ok {
+			return &meta, nil
+		}
 	}
 	return nil, ErrMissingMeta{Role: role}
 }

tuf/data/types.go

@@ -4,6 +4,7 @@ import (
 	"crypto/sha256"
 	"crypto/sha512"
 	"crypto/subtle"
+	"encoding/hex"
 	"fmt"
 	"hash"
 	"io"
@@ -132,7 +133,7 @@ type FileMeta struct {
 }
 
 // CheckHashes verifies all the checksums specified by the "hashes" of the payload.
-func CheckHashes(payload []byte, hashes Hashes) error {
+func CheckHashes(payload []byte, name string, hashes Hashes) error {
 	cnt := 0
 
 	// k, v indicate the hash algorithm and the corresponding value
@@ -141,20 +142,20 @@ func CheckHashes(payload []byte, hashes Hashes) error {
 		case notary.SHA256:
 			checksum := sha256.Sum256(payload)
 			if subtle.ConstantTimeCompare(checksum[:], v) == 0 {
-				return ErrMismatchedChecksum{alg: notary.SHA256}
+				return ErrMismatchedChecksum{alg: notary.SHA256, name: name, expected: hex.EncodeToString(v)}
 			}
 			cnt++
 		case notary.SHA512:
 			checksum := sha512.Sum512(payload)
 			if subtle.ConstantTimeCompare(checksum[:], v) == 0 {
-				return ErrMismatchedChecksum{alg: notary.SHA512}
+				return ErrMismatchedChecksum{alg: notary.SHA512, name: name, expected: hex.EncodeToString(v)}
 			}
 			cnt++
 		}
 	}
 
 	if cnt == 0 {
-		return fmt.Errorf("at least one supported hash needed")
+		return ErrMissingMeta{Role: name}
 	}
 
 	return nil

tuf/data/types_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/docker/go/canonical/json"
+	"github.com/docker/notary"
 	"github.com/stretchr/testify/require"
 )
 
@@ -62,63 +63,81 @@ func TestCheckHashes(t *testing.T) {
 	// it should be considered as fail.
 	unSupported := make(Hashes)
 	unSupported["Arthas"] = []byte("is past away.")
-	err = CheckHashes(raw, unSupported)
+	err = CheckHashes(raw, "metaName1", unSupported)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "at least one supported hash needed")
+	missingMeta, ok := err.(ErrMissingMeta)
+	require.True(t, ok)
+	require.Equal(t, "metaName1", missingMeta.Role)
 
 	// Expected to fail since there is no checksum at all.
 	hashes := make(Hashes)
-	err = CheckHashes(raw, hashes)
+	err = CheckHashes(raw, "metaName2", hashes)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "at least one supported hash needed")
+	missingMeta, ok = err.(ErrMissingMeta)
+	require.True(t, ok)
+	require.Equal(t, "metaName2", missingMeta.Role)
 
 	// The most standard one.
-	hashes["sha256"], err = hex.DecodeString("d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
+	hashes[notary.SHA256], err = hex.DecodeString("d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
 	require.NoError(t, err)
-	hashes["sha512"], err = hex.DecodeString("f2330f50d0f3ee56cf0d7f66aad8205e0cb9972c323208ffaa914ef7b3c240ae4774b5bbd1db2ce226ee967cfa9058173a853944f9b44e2e08abca385e2b7ed4")
+	hashes[notary.SHA512], err = hex.DecodeString("f2330f50d0f3ee56cf0d7f66aad8205e0cb9972c323208ffaa914ef7b3c240ae4774b5bbd1db2ce226ee967cfa9058173a853944f9b44e2e08abca385e2b7ed4")
 	require.NoError(t, err)
-	err = CheckHashes(raw, hashes)
+	err = CheckHashes(raw, "meta", hashes)
 	require.NoError(t, err)
 
 	// Expected as success since there are already supported hash here,
 	// just ignore the unsupported one.
 	hashes["Saar"] = []byte("survives again in CTM.")
-	err = CheckHashes(raw, hashes)
+	err = CheckHashes(raw, "meta", hashes)
 	require.NoError(t, err)
 
 	only256 := make(Hashes)
-	only256["sha256"], err = hex.DecodeString("d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
+	only256[notary.SHA256], err = hex.DecodeString("d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
 	require.NoError(t, err)
-	err = CheckHashes(raw, only256)
+	err = CheckHashes(raw, "meta", only256)
 	require.NoError(t, err)
 
 	only512 := make(Hashes)
 	only512["sha512"], err = hex.DecodeString("f2330f50d0f3ee56cf0d7f66aad8205e0cb9972c323208ffaa914ef7b3c240ae4774b5bbd1db2ce226ee967cfa9058173a853944f9b44e2e08abca385e2b7ed4")
 	require.NoError(t, err)
-	err = CheckHashes(raw, only512)
+	err = CheckHashes(raw, "meta", only512)
 	require.NoError(t, err)
 
 	// Expected to fail due to the failure of sha256
 	malicious256 := make(Hashes)
-	malicious256["sha256"] = []byte("malicious data")
-	err = CheckHashes(raw, malicious256)
-	require.IsType(t, ErrMismatchedChecksum{}, err)
+	malicious256[notary.SHA256] = []byte("malicious data")
+	err = CheckHashes(raw, "metaName3", malicious256)
+	require.Error(t, err)
+	badChecksum, ok := err.(ErrMismatchedChecksum)
+	require.True(t, ok)
+	require.Equal(t, ErrMismatchedChecksum{alg: notary.SHA256, name: "metaName3",
+		expected: hex.EncodeToString([]byte("malicious data"))}, badChecksum)
 
 	// Expected to fail due to the failure of sha512
 	malicious512 := make(Hashes)
-	malicious512["sha512"] = []byte("malicious data")
-	err = CheckHashes(raw, malicious512)
-	require.IsType(t, ErrMismatchedChecksum{}, err)
+	malicious512[notary.SHA512] = []byte("malicious data")
+	err = CheckHashes(raw, "metaName4", malicious512)
+	require.Error(t, err)
+	badChecksum, ok = err.(ErrMismatchedChecksum)
+	require.True(t, ok)
+	require.Equal(t, ErrMismatchedChecksum{alg: notary.SHA512, name: "metaName4",
+		expected: hex.EncodeToString([]byte("malicious data"))}, badChecksum)
 
 	// Expected to fail because of the failure of sha512
 	// even though the sha256 is OK.
 	doubleFace := make(Hashes)
-	doubleFace["sha256"], err = hex.DecodeString("d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
+	doubleFace[notary.SHA256], err = hex.DecodeString(
+		"d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
 	require.NoError(t, err)
-	doubleFace["sha512"], err = hex.DecodeString("d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
+	doubleFace[notary.SHA512], err = hex.DecodeString(
+		"d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92")
 	require.NoError(t, err)
-	err = CheckHashes(raw, doubleFace)
-	require.IsType(t, ErrMismatchedChecksum{}, err)
+	err = CheckHashes(raw, "metaName5", doubleFace)
+	require.Error(t, err)
+	badChecksum, ok = err.(ErrMismatchedChecksum)
+	require.True(t, ok)
+	require.Equal(t, ErrMismatchedChecksum{alg: notary.SHA512, name: "metaName5",
+		expected: "d13e2b60d74c2e6f4f449b5e536814edf9a4827f5a9f4f957fc92e77609b9c92"}, badChecksum)
 }
 
 func TestCheckValidHashStructures(t *testing.T) {