Remove signed.Verify in favor of just using signed.VerifySignatures,

signed.VerifyVersion, and signed.VerifyExpiry.

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

tuf/signed/verify.go

@@ -21,33 +21,6 @@ var (
 	ErrWrongType    = errors.New("tuf: meta file has wrong type")
 )
 
-// Verify checks the signatures and metadata (expiry, version) for the signed role
-// data
-func Verify(s *data.Signed, role data.BaseRole, minVersion int) error {
-	if err := verifyMeta(s, role.Name, minVersion); err != nil {
-		return err
-	}
-	return VerifySignatures(s, role)
-}
-
-func verifyMeta(s *data.Signed, role string, minVersion int) error {
-	sm := &data.SignedCommon{}
-	if err := json.Unmarshal(*s.Signed, sm); err != nil {
-		return err
-	}
-	if !data.ValidTUFType(sm.Type, role) {
-		return ErrWrongType
-	}
-	if err := VerifyExpiry(sm, role); err != nil {
-		return err
-	}
-	if err := VerifyVersion(sm, minVersion); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // IsExpired checks if the given time passed before the present time
 func IsExpired(t time.Time) bool {
 	return t.Before(time.Now())
@@ -65,7 +38,7 @@ func VerifyExpiry(s *data.SignedCommon, role string) error {
 // VerifyVersion returns ErrLowVersion if the metadata version is lower than the min version
 func VerifyVersion(s *data.SignedCommon, minVersion int) error {
 	if s.Version < minVersion {
-		return ErrLowVersion{s.Version, minVersion}
+		return ErrLowVersion{Actual: s.Version, Current: minVersion}
 	}
 	return nil
 }

tuf/signed/verify_test.go

@@ -1,11 +1,11 @@
 package signed
 
 import (
-	"errors"
 	"testing"
 	"time"
 
 	"github.com/docker/go/canonical/json"
+	"github.com/docker/notary"
 	"github.com/stretchr/testify/require"
 
 	"github.com/docker/notary/tuf/data"
@@ -23,8 +23,8 @@ func TestRoleNoKeys(t *testing.T) {
 	b, err := json.MarshalCanonical(meta)
 	require.NoError(t, err)
 	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-	Sign(cs, s, []data.PublicKey{k}, 1, nil)
-	err = Verify(s, roleWithKeys, 1)
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k}, 1, nil))
+	err = VerifySignatures(s, roleWithKeys)
 	require.IsType(t, ErrRoleThreshold{}, err)
 }
 
@@ -40,11 +40,45 @@ func TestNotEnoughSigs(t *testing.T) {
 	b, err := json.MarshalCanonical(meta)
 	require.NoError(t, err)
 	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-	Sign(cs, s, []data.PublicKey{k}, 1, nil)
-	err = Verify(s, roleWithKeys, 1)
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k}, 1, nil))
+	err = VerifySignatures(s, roleWithKeys)
 	require.IsType(t, ErrRoleThreshold{}, err)
 }
 
+func TestNoSigs(t *testing.T) {
+	cs := NewEd25519()
+	k, err := cs.Create("root", "", data.ED25519Key)
+	require.NoError(t, err)
+	require.NoError(t, err)
+	roleWithKeys := data.BaseRole{Name: "root", Keys: data.Keys{k.ID(): k}, Threshold: 2}
+
+	meta := &data.SignedCommon{Type: "Root", Version: 1, Expires: data.DefaultExpires("root")}
+
+	b, err := json.MarshalCanonical(meta)
+	require.NoError(t, err)
+	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
+	require.Equal(t, ErrNoSignatures, VerifySignatures(s, roleWithKeys))
+}
+
+func TestExactlyEnoughSigs(t *testing.T) {
+	cs := NewEd25519()
+	k, err := cs.Create(data.CanonicalRootRole, "", data.ED25519Key)
+	require.NoError(t, err)
+	roleWithKeys := data.BaseRole{
+		Name: data.CanonicalRootRole, Keys: data.Keys{k.ID(): k}, Threshold: 1}
+
+	meta := &data.SignedCommon{Type: data.TUFTypes[data.CanonicalRootRole], Version: 1,
+		Expires: data.DefaultExpires(data.CanonicalRootRole)}
+
+	b, err := json.MarshalCanonical(meta)
+	require.NoError(t, err)
+	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k}, 1, nil))
+	require.Equal(t, 1, len(s.Signatures))
+
+	require.NoError(t, VerifySignatures(s, roleWithKeys))
+}
+
 func TestMoreThanEnoughSigs(t *testing.T) {
 	cs := NewEd25519()
 	k1, err := cs.Create("root", "", data.ED25519Key)
@@ -58,10 +92,10 @@ func TestMoreThanEnoughSigs(t *testing.T) {
 	b, err := json.MarshalCanonical(meta)
 	require.NoError(t, err)
 	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-	Sign(cs, s, []data.PublicKey{k1, k2}, 2, nil)
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k1, k2}, 2, nil))
 	require.Equal(t, 2, len(s.Signatures))
 
-	err = Verify(s, roleWithKeys, 1)
+	err = VerifySignatures(s, roleWithKeys)
 	require.NoError(t, err)
 }
 
@@ -76,10 +110,10 @@ func TestValidSigWithIncorrectKeyID(t *testing.T) {
 	b, err := json.MarshalCanonical(meta)
 	require.NoError(t, err)
 	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-	Sign(cs, s, []data.PublicKey{k1}, 1, nil)
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k1}, 1, nil))
 	require.Equal(t, 1, len(s.Signatures))
 	s.Signatures[0].KeyID = "invalidIDA"
-	err = Verify(s, roleWithKeys, 1)
+	err = VerifySignatures(s, roleWithKeys)
 	require.Error(t, err)
 	require.IsType(t, ErrInvalidKeyID{}, err)
 }
@@ -95,9 +129,9 @@ func TestDuplicateSigs(t *testing.T) {
 	b, err := json.MarshalCanonical(meta)
 	require.NoError(t, err)
 	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-	Sign(cs, s, []data.PublicKey{k}, 1, nil)
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k}, 1, nil))
 	s.Signatures = append(s.Signatures, s.Signatures[0])
-	err = Verify(s, roleWithKeys, 1)
+	err = VerifySignatures(s, roleWithKeys)
 	require.IsType(t, ErrRoleThreshold{}, err)
 }
 
@@ -114,101 +148,28 @@ func TestUnknownKeyBelowThreshold(t *testing.T) {
 	b, err := json.MarshalCanonical(meta)
 	require.NoError(t, err)
 	s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-	Sign(cs, s, []data.PublicKey{k, unknown}, 2, nil)
+	require.NoError(t, Sign(cs, s, []data.PublicKey{k, unknown}, 2, nil))
 	s.Signatures = append(s.Signatures)
-	err = Verify(s, roleWithKeys, 1)
+	err = VerifySignatures(s, roleWithKeys)
 	require.IsType(t, ErrRoleThreshold{}, err)
 }
 
-func Test(t *testing.T) {
-	cryptoService := NewEd25519()
-	type test struct {
-		name     string
-		roleData data.BaseRole
-		s        *data.Signed
-		ver      int
-		exp      *time.Time
-		typ      string
-		role     string
-		err      error
-		mut      func(*test)
-	}
-
-	expiredTime := time.Now().Add(-time.Hour)
-	minVer := 10
-	tests := []test{
-		{
-			name: "no signatures",
-			mut:  func(t *test) { t.s.Signatures = []data.Signature{} },
-			err:  ErrNoSignatures,
-		},
-		{
-			name: "unknown role",
-			role: "foo",
-			err:  errors.New("tuf: meta file has wrong type"),
-		},
-		{
-			name: "exactly enough signatures",
-		},
-		{
-			name: "wrong type",
-			typ:  "bar",
-			err:  ErrWrongType,
-		},
-		{
-			name: "low version",
-			ver:  minVer - 1,
-			err:  ErrLowVersion{minVer - 1, minVer},
-		},
-		{
-			role: "root",
-			name: "expired",
-			exp:  &expiredTime,
-			err:  ErrExpired{"root", expiredTime.Format("Mon Jan 2 15:04:05 MST 2006")},
-		},
-	}
-	for _, run := range tests {
-		if run.role == "" {
-			run.role = "root"
-		}
-		if run.ver == 0 {
-			run.ver = minVer
-		}
-		if run.exp == nil {
-			expires := time.Now().Add(time.Hour)
-			run.exp = &expires
-		}
-		if run.typ == "" {
-			run.typ = data.TUFTypes[run.role]
-		}
-		if run.s == nil {
-			k, _ := cryptoService.Create("root", "", data.ED25519Key)
-			run.roleData = data.BaseRole{Name: "root", Keys: data.Keys{k.ID(): k}, Threshold: 1}
-			meta := &data.SignedCommon{Type: run.typ, Version: run.ver, Expires: *run.exp}
-
-			b, err := json.MarshalCanonical(meta)
-			require.NoError(t, err)
-			s := &data.Signed{Signed: (*json.RawMessage)(&b)}
-			Sign(cryptoService, s, []data.PublicKey{k}, 1, nil)
-			run.s = s
-		}
-		if run.mut != nil {
-			run.mut(&run)
-		}
-
-		err := Verify(run.s, run.roleData, minVer)
-		if e, ok := run.err.(ErrExpired); ok {
-			requireErrExpired(t, err, e)
-		} else {
-			require.Equal(t, run.err, err)
-		}
-	}
+func TestVerifyVersion(t *testing.T) {
+	tufType := data.TUFTypes[data.CanonicalRootRole]
+	meta := data.SignedCommon{Type: tufType, Version: 1, Expires: data.DefaultExpires(data.CanonicalRootRole)}
+	require.Equal(t, ErrLowVersion{Actual: 1, Current: 2}, VerifyVersion(&meta, 2))
+	require.NoError(t, VerifyVersion(&meta, 1))
 }
 
-func requireErrExpired(t *testing.T, err error, expected ErrExpired) {
-	actual, ok := err.(ErrExpired)
-	if !ok {
-		t.Fatalf("expected err to have type ErrExpired, got %T", err)
-	}
-	require.Equal(t, actual.Expired, expected.Expired)
+func TestVerifyExpiry(t *testing.T) {
+	tufType := data.TUFTypes[data.CanonicalRootRole]
+	notExpired := data.DefaultExpires(data.CanonicalRootRole)
+	expired := time.Now().Add(-1 * notary.Year)
+
+	require.NoError(t, VerifyExpiry(
+		&data.SignedCommon{Type: tufType, Version: 1, Expires: notExpired}, data.CanonicalRootRole))
+	err := VerifyExpiry(
+		&data.SignedCommon{Type: tufType, Version: 1, Expires: expired}, data.CanonicalRootRole)
+	require.Error(t, err)
+	require.IsType(t, ErrExpired{}, err)
 }

tuf/testutils/swizzler_test.go

@@ -420,7 +420,8 @@ func TestSwizzlerChangeRootKey(t *testing.T) {
 			require.NoError(t, err)
 			signedThing, err := newRoot.ToSigned()
 			require.NoError(t, err)
-			require.NoError(t, signed.Verify(signedThing, rootRole, 1))
+			require.NoError(t, signed.VerifySignatures(signedThing, rootRole))
+			require.NoError(t, signed.VerifyVersion(&(newRoot.Signed.SignedCommon), 1))
 		default:
 			require.True(t, bytes.Equal(origMeta, newMeta), "bytes have changed for role %s", role)
 		}