Add root metadata validation to the root data structure

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

certs/certs_test.go

@@ -285,7 +285,11 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string
 
 	testRoot, err := data.NewRoot(
 		map[string]data.PublicKey{replRootKey.ID(): replRootKey},
-		map[string]*data.RootRole{data.CanonicalRootRole: &rootRole.RootRole},
+		map[string]*data.RootRole{
+			data.CanonicalRootRole:      &rootRole.RootRole,
+			data.CanonicalTimestampRole: &rootRole.RootRole,
+			data.CanonicalTargetsRole:   &rootRole.RootRole,
+			data.CanonicalSnapshotRole:  &rootRole.RootRole},
 		false,
 	)
 	assert.NoError(t, err, "Failed to create new root")

client/client_update_test.go

@@ -783,7 +783,7 @@ var waysToMessUpServer = []swizzleExpectations{
 	// for everything else, the errors come from tuf/signed
 
 	{desc: "invalid SignedMeta Type", expectErrs: []interface{}{
-		&certs.ErrValidationFail{}, signed.ErrWrongType},
+		&certs.ErrValidationFail{}, signed.ErrWrongType, data.ErrInvalidMeta{}},
 		swizzle: (*testutils.MetadataSwizzler).SetInvalidMetadataType},
 
 	{desc: "invalid signatures", expectErrs: []interface{}{

server/handlers/validation_test.go

@@ -482,7 +482,7 @@ func TestValidateRootInvalidZeroThreshold(t *testing.T) {
 		copyTimestampKey(t, oldRepo, store, "testGUN")
 		_, err = validateUpdate(cs, "testGUN", updates, store)
 		assert.Error(t, err)
-		assert.Contains(t, err.Error(), "role has invalid threshold")
+		assert.Contains(t, err.Error(), "invalid threshold")
 	}
 }
 

tuf/client/client_test.go

@@ -47,7 +47,12 @@ func TestRotation(t *testing.T) {
 	// Generate a new root with the replacement key and role
 	testRoot, err := data.NewRoot(
 		map[string]data.PublicKey{replacementKey.ID(): replacementKey},
-		map[string]*data.RootRole{"root": &replacementRole.RootRole},
+		map[string]*data.RootRole{
+			data.CanonicalRootRole:      &replacementRole.RootRole,
+			data.CanonicalSnapshotRole:  &replacementRole.RootRole,
+			data.CanonicalTargetsRole:   &replacementRole.RootRole,
+			data.CanonicalTimestampRole: &replacementRole.RootRole,
+		},
 		false,
 	)
 	assert.NoError(t, err, "Failed to create new root")

tuf/data/root.go

@@ -1,6 +1,7 @@
 package data
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/docker/go/canonical/json"
@@ -23,14 +24,51 @@ type Root struct {
 	ConsistentSnapshot bool                 `json:"consistent_snapshot"`
 }
 
+// isValidRoot returns an error, or nil, depending on whether the content of the struct
+// is valid for root metadata.  This does not check signatures or expiry, just that
+// the metadata content is valid.
+func isValidRoot(r Root) error {
+	expectedType := TUFTypes[CanonicalRootRole]
+	if r.Type != expectedType {
+		return ErrInvalidMeta{
+			Role: CanonicalRootRole, Msg: fmt.Sprintf("expected type %s, not %s", expectedType, r.Type)}
+	}
+	if len(r.Roles) < 4 {
+		return ErrInvalidMeta{Role: CanonicalRootRole, Msg: "does not have all required roles"}
+	} else if len(r.Roles) > 4 {
+		return ErrInvalidMeta{Role: CanonicalRootRole, Msg: "specifies too many roles"}
+	}
+
+	for _, roleName := range BaseRoles {
+		roleObj, ok := r.Roles[roleName]
+		if !ok || roleObj == nil {
+			return ErrInvalidMeta{
+				Role: CanonicalRootRole, Msg: fmt.Sprintf("missing %s role specification", roleName)}
+		}
+		if roleObj.Threshold < 1 {
+			return ErrInvalidMeta{
+				Role: CanonicalRootRole, Msg: fmt.Sprintf("invalid threshold: %v", roleObj.Threshold)}
+		}
+		for _, keyID := range roleObj.KeyIDs {
+			if _, ok := r.Keys[keyID]; !ok {
+				return ErrInvalidMeta{
+					Role: CanonicalRootRole,
+					Msg:  fmt.Sprintf("%s role specifies key ID %s without corresponding key", roleName, keyID),
+				}
+			}
+		}
+	}
+	return nil
+}
+
 // NewRoot initializes a new SignedRoot with a set of keys, roles, and the consistent flag
 func NewRoot(keys map[string]PublicKey, roles map[string]*RootRole, consistent bool) (*SignedRoot, error) {
 	signedRoot := &SignedRoot{
 		Signatures: make([]Signature, 0),
 		Signed: Root{
-			Type:               TUFTypes["root"],
+			Type:               TUFTypes[CanonicalRootRole],
 			Version:            0,
-			Expires:            DefaultExpires("root"),
+			Expires:            DefaultExpires(CanonicalRootRole),
 			Keys:               keys,
 			Roles:              roles,
 			ConsistentSnapshot: consistent,
@@ -42,7 +80,7 @@ func NewRoot(keys map[string]PublicKey, roles map[string]*RootRole, consistent b
 }
 
 // ToSigned partially serializes a SignedRoot for further signing
-func (r SignedRoot) ToSigned() (*Signed, error) {
+func (r *SignedRoot) ToSigned() (*Signed, error) {
 	s, err := defaultSerializer.MarshalCanonical(r.Signed)
 	if err != nil {
 		return nil, err
@@ -62,7 +100,7 @@ func (r SignedRoot) ToSigned() (*Signed, error) {
 }
 
 // MarshalJSON returns the serialized form of SignedRoot as bytes
-func (r SignedRoot) MarshalJSON() ([]byte, error) {
+func (r *SignedRoot) MarshalJSON() ([]byte, error) {
 	signed, err := r.ToSigned()
 	if err != nil {
 		return nil, err
@@ -70,11 +108,14 @@ func (r SignedRoot) MarshalJSON() ([]byte, error) {
 	return defaultSerializer.Marshal(signed)
 }
 
-// RootFromSigned fully unpacks a Signed object into a SignedRoot
+// RootFromSigned fully unpacks a Signed object into a SignedRoot and ensures
+// that it is a valid SignedRoot
 func RootFromSigned(s *Signed) (*SignedRoot, error) {
 	r := Root{}
-	err := json.Unmarshal(s.Signed, &r)
-	if err != nil {
+	if err := json.Unmarshal(s.Signed, &r); err != nil {
+		return nil, err
+	}
+	if err := isValidRoot(r); err != nil {
 		return nil, err
 	}
 	sigs := make([]Signature, len(s.Signatures))

tuf/data/root_test.go

@@ -20,7 +20,34 @@ func (e errorSerializer) MarshalCanonical(from interface{}) ([]byte, error) {
 	return nil, fmt.Errorf("bad")
 }
 
-func TestToSignedMarshalsSignedPortionWithCanonicalJSON(t *testing.T) {
+func validRootTemplate() *SignedRoot {
+	return &SignedRoot{
+		Signed: Root{
+			Type:    "Root",
+			Version: 1,
+			Expires: time.Now(),
+			Keys: Keys{
+				"key1":  NewPublicKey(RSAKey, []byte("key1")),
+				"key2":  NewPublicKey(RSAKey, []byte("key2")),
+				"key3":  NewPublicKey(RSAKey, []byte("key3")),
+				"snKey": NewPublicKey(RSAKey, []byte("snKey")),
+				"tgKey": NewPublicKey(RSAKey, []byte("tgKey")),
+				"tsKey": NewPublicKey(RSAKey, []byte("tsKey")),
+			},
+			Roles: map[string]*RootRole{
+				CanonicalRootRole:      {KeyIDs: []string{"key1"}, Threshold: 1},
+				CanonicalSnapshotRole:  {KeyIDs: []string{"snKey"}, Threshold: 1},
+				CanonicalTimestampRole: {KeyIDs: []string{"tsKey"}, Threshold: 1},
+				CanonicalTargetsRole:   {KeyIDs: []string{"tgKey"}, Threshold: 1},
+			},
+		},
+		Signatures: []Signature{
+			{KeyID: "key1", Method: "method1", Signature: []byte("hello")},
+		},
+	}
+}
+
+func TestRootToSignedMarshalsSignedPortionWithCanonicalJSON(t *testing.T) {
 	r := SignedRoot{Signed: Root{Type: "root", Version: 2, Expires: time.Now()}}
 	signedCanonical, err := r.ToSigned()
 	require.NoError(t, err)
@@ -36,7 +63,7 @@ func TestToSignedMarshalsSignedPortionWithCanonicalJSON(t *testing.T) {
 		"expected %v == %v", signedCanonical.Signed, castedCanonical)
 }
 
-func TestToSignCopiesSignatures(t *testing.T) {
+func TestRootToSignCopiesSignatures(t *testing.T) {
 	r := SignedRoot{
 		Signed: Root{Type: "root", Version: 2, Expires: time.Now()},
 		Signatures: []Signature{
@@ -54,7 +81,7 @@ func TestToSignCopiesSignatures(t *testing.T) {
 		"expected %v != %v", r.Signatures, signed.Signatures)
 }
 
-func TestToSignedMarshallingErrorsPropagated(t *testing.T) {
+func TestRootToSignedMarshallingErrorsPropagated(t *testing.T) {
 	setDefaultSerializer(errorSerializer{})
 	defer setDefaultSerializer(canonicalJSON{})
 	r := SignedRoot{
@@ -64,7 +91,7 @@ func TestToSignedMarshallingErrorsPropagated(t *testing.T) {
 	require.EqualError(t, err, "bad")
 }
 
-func TestMarshalJSONMarshalsSignedWithRegularJSON(t *testing.T) {
+func TestRootMarshalJSONMarshalsSignedWithRegularJSON(t *testing.T) {
 	r := SignedRoot{
 		Signed: Root{Type: "root", Version: 2, Expires: time.Now()},
 		Signatures: []Signature{
@@ -87,7 +114,7 @@ func TestMarshalJSONMarshalsSignedWithRegularJSON(t *testing.T) {
 		"expected %v != %v", serialized, regular)
 }
 
-func TestMarshalJSONMarshallingErrorsPropagated(t *testing.T) {
+func TestRootMarshalJSONMarshallingErrorsPropagated(t *testing.T) {
 	setDefaultSerializer(errorSerializer{})
 	defer setDefaultSerializer(canonicalJSON{})
 	r := SignedRoot{
@@ -96,3 +123,83 @@ func TestMarshalJSONMarshallingErrorsPropagated(t *testing.T) {
 	_, err := r.MarshalJSON()
 	require.EqualError(t, err, "bad")
 }
+
+// RootFromSigned succeeds if the root is valid, and copies the signatures
+// rather than assigns them
+func TestRootFromSignedCopiesSignatures(t *testing.T) {
+	signed, err := validRootTemplate().ToSigned()
+	require.NoError(t, err)
+
+	signedRoot, err := RootFromSigned(signed)
+	require.NoError(t, err)
+
+	signed.Signatures[0] = Signature{KeyID: "key3", Method: "method3", Signature: []byte("world")}
+
+	require.Equal(t, "key3", signed.Signatures[0].KeyID)
+	require.Equal(t, "key1", signedRoot.Signatures[0].KeyID)
+}
+
+func rootToSignedAndBack(t *testing.T, root *SignedRoot) (*SignedRoot, error) {
+	s, err := root.ToSigned()
+	require.NoError(t, err)
+	return RootFromSigned(s)
+}
+
+// If the role data specified is nil, or has an invalid threshold, or doesn't have enough
+// keys to cover the threshold, or has key IDs that are not in the key list, the root
+// metadata fails to validate and thus fails to convert into a SignedRoot
+func TestRootFromSignedValidatesRoleData(t *testing.T) {
+	var err error
+	for _, roleName := range BaseRoles {
+		root := validRootTemplate()
+
+		// Invalid threshold
+		root.Signed.Roles[roleName].Threshold = 0
+		_, err = rootToSignedAndBack(t, root)
+		require.IsType(t, ErrInvalidMeta{}, err)
+
+		// Keys that aren't in the list of keys
+		root.Signed.Roles[roleName].Threshold = 1
+		root.Signed.Roles[roleName].KeyIDs = []string{"key11"}
+		_, err = rootToSignedAndBack(t, root)
+		require.IsType(t, ErrInvalidMeta{}, err)
+
+		// role is nil
+		root.Signed.Roles[roleName] = nil
+		_, err = rootToSignedAndBack(t, root)
+		require.IsType(t, ErrInvalidMeta{}, err)
+
+		// too few roles
+		delete(root.Signed.Roles, roleName)
+		_, err = rootToSignedAndBack(t, root)
+		require.IsType(t, ErrInvalidMeta{}, err)
+
+		// add an extra role that doesn't belong, so that the number of roles
+		// is correct a required one is still missing
+		root.Signed.Roles["extraneous"] = &RootRole{KeyIDs: []string{"key3"}, Threshold: 1}
+		_, err = rootToSignedAndBack(t, root)
+		require.IsType(t, ErrInvalidMeta{}, err)
+	}
+
+	// Add an extra role without messing with the others
+	root := validRootTemplate()
+	root.Signed.Roles["extraneous"] = &RootRole{KeyIDs: []string{"key3"}, Threshold: 1}
+	_, err = rootToSignedAndBack(t, root)
+	require.IsType(t, ErrInvalidMeta{}, err)
+}
+
+// The type must be "Root"
+func TestRootFromSignedValidatesRoleType(t *testing.T) {
+	root := validRootTemplate()
+
+	for _, invalid := range []string{"Root ", CanonicalSnapshotRole, "rootroot", "RoOt", "root"} {
+		root.Signed.Type = invalid
+		_, err := rootToSignedAndBack(t, root)
+		require.IsType(t, ErrInvalidMeta{}, err)
+	}
+
+	root.Signed.Type = "Root"
+	sRoot, err := rootToSignedAndBack(t, root)
+	require.NoError(t, err)
+	require.Equal(t, "Root", sRoot.Signed.Type)
+}

tuf/data/types.go

@@ -212,3 +212,13 @@ func (s *Signature) UnmarshalJSON(data []byte) error {
 	*s = Signature(uSignature)
 	return nil
 }
+
+// ErrInvalidMeta is the error to be returned when metadata is invalid
+type ErrInvalidMeta struct {
+	Role string
+	Msg  string
+}
+
+func (e ErrInvalidMeta) Error() string {
+	return fmt.Sprintf("%s metadata invalid: %s", e.Role, e.Msg)
+}