diff --git a/client/client_update_test.go b/client/client_update_test.go index 10279c08c3..80052d97f1 100644 --- a/client/client_update_test.go +++ b/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:] diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index ca6860914f..36e0630371 100644 --- a/cmd/notary/tuf.go +++ b/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) } diff --git a/server/timestamp/timestamp.go b/server/timestamp/timestamp.go index 32d1bbdbc2..5e536505d5 100644 --- a/server/timestamp/timestamp.go +++ b/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 diff --git a/tuf/client/client.go b/tuf/client/client.go index 31f8d4b3fe..349f820b02 100644 --- a/tuf/client/client.go +++ b/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 } diff --git a/tuf/client/client_test.go b/tuf/client/client_test.go index 5e3fb78dc0..02160086b8 100644 --- a/tuf/client/client_test.go +++ b/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) { diff --git a/tuf/client/errors.go b/tuf/client/errors.go index ad0555127e..28a6836afd 100644 --- a/tuf/client/errors.go +++ b/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 diff --git a/tuf/data/errors.go b/tuf/data/errors.go index c648866f60..0cd0afba06 100644 --- a/tuf/data/errors.go +++ b/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) } diff --git a/tuf/data/snapshot.go b/tuf/data/snapshot.go index 172a1465e9..f77b7a2549 100644 --- a/tuf/data/snapshot.go +++ b/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} } diff --git a/tuf/data/types.go b/tuf/data/types.go index a664e0d9b1..6b61f4e192 100644 --- a/tuf/data/types.go +++ b/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 diff --git a/tuf/data/types_test.go b/tuf/data/types_test.go index 9353db67ba..5bb980bc80 100644 --- a/tuf/data/types_test.go +++ b/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) {