Merge pull request #300 from docker/server-validate-timestamp-key

Server check that the root.json's timestamp key ID is valid.

server/handlers/validation.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/docker/notary/tuf"
 	"github.com/docker/notary/tuf/data"
@@ -94,10 +95,11 @@ func validateUpdate(gun string, updates []storage.MetaUpdate, store storage.Meta
 	if rootUpdate, ok := roles[rootRole]; ok {
 		// if root is present, validate its integrity, possibly
 		// against a previous root
-		if root, err = validateRoot(gun, oldRootJSON, rootUpdate.Data); err != nil {
+		if root, err = validateRoot(gun, oldRootJSON, rootUpdate.Data, store); err != nil {
 			logrus.Error("ErrBadRoot: ", err.Error())
 			return ErrBadRoot{msg: err.Error()}
 		}
+
 		// setting root will update keys db
 		if err = repo.SetRoot(root); err != nil {
 			logrus.Error("ErrValidation: ", err.Error())
@@ -252,7 +254,9 @@ func hierarchyOK(roles map[string]storage.MetaUpdate) error {
 	return nil
 }
 
-func validateRoot(gun string, oldRoot, newRoot []byte) (*data.SignedRoot, error) {
+func validateRoot(gun string, oldRoot, newRoot []byte, store storage.MetaStore) (
+	*data.SignedRoot, error) {
+
 	var parsedOldRoot *data.SignedRoot
 	parsedNewRoot := &data.SignedRoot{}
 
@@ -271,23 +275,32 @@ func validateRoot(gun string, oldRoot, newRoot []byte) (*data.SignedRoot, error)
 	if err != nil {
 		return nil, err
 	}
-	if err := checkRoot(parsedOldRoot, parsedNewRoot); err != nil {
+
+	// Don't update if a timestamp key doesn't exist.
+	algo, keyBytes, err := store.GetTimestampKey(gun)
+	if err != nil || algo == "" || keyBytes == nil {
+		return nil, fmt.Errorf("no timestamp key for %s", gun)
+	}
+	timestampKey := data.NewPublicKey(algo, keyBytes)
+
+	if err := checkRoot(parsedOldRoot, parsedNewRoot, timestampKey); err != nil {
 		// TODO(david): how strict do we want to be here about old signatures
 		//              for rotations? Should the user have to provide a flag
 		//              which gets transmitted to force a root update without
 		//              correct old key signatures.
 		return nil, err
 	}
+
 	if !data.ValidTUFType(parsedNewRoot.Signed.Type, data.CanonicalRootRole) {
 		return nil, fmt.Errorf("root has wrong type")
 	}
 	return parsedNewRoot, nil
 }
 
-// checkRoot returns true if no rotation, or a valid
+// checkRoot errors if an invalid rotation has taken place, if the
-// rotation has taken place, and the threshold number of signatures
+// threshold number of signatures is invalid, if there are an invalid
-// are valid.
+// number of roles and keys, or if the timestamp keys are invalid
-func checkRoot(oldRoot, newRoot *data.SignedRoot) error {
+func checkRoot(oldRoot, newRoot *data.SignedRoot, timestampKey data.PublicKey) error {
 	rootRole := data.RoleName(data.CanonicalRootRole)
 	targetsRole := data.RoleName(data.CanonicalTargetsRole)
 	snapshotRole := data.RoleName(data.CanonicalSnapshotRole)
@@ -360,18 +373,39 @@ func checkRoot(oldRoot, newRoot *data.SignedRoot) error {
 	if err != nil {
 		return err
 	}
+
+	var timestampKeyIDs []string
+
 	// at a minimum, check the 4 required roles are present
 	for _, r := range []string{rootRole, targetsRole, snapshotRole, timestampRole} {
 		role, ok := root.Signed.Roles[r]
 		if !ok {
 			return fmt.Errorf("missing required %s role from root", r)
 		}
-		if role.Threshold < 1 {
+		// According to the TUF spec, any role may have more than one signing
+		// key and require a threshold signature.  However, notary-server
+		// creates the timestamp, and there is only ever one, so a threshold
+		// greater than one would just always fail validation
+		if (r == timestampRole && role.Threshold != 1) || role.Threshold < 1 {
 			return fmt.Errorf("%s role has invalid threshold", r)
 		}
 		if len(role.KeyIDs) < role.Threshold {
 			return fmt.Errorf("%s role has insufficient number of keys", r)
 		}
+
+		if r == timestampRole {
+			timestampKeyIDs = role.KeyIDs
 		}
+	}
+
+	// ensure that at least one of the timestamp keys specified in the role
+	// actually exists
+
+	for _, keyID := range timestampKeyIDs {
+		if timestampKey.ID() == keyID {
 			return nil
 		}
+	}
+	return fmt.Errorf("none of the following timestamp keys exist: %s",
+		strings.Join(timestampKeyIDs, ", "))
+}

tuf/store/httpstore.go

@@ -33,6 +33,19 @@ func (err ErrMaliciousServer) Error() string {
 	return "Trust server returned a bad response."
 }
 
+// ErrInvalidOperation indicates that the server returned a 400 response and
+// propogate any body we received.
+type ErrInvalidOperation struct {
+	msg string
+}
+
+func (err ErrInvalidOperation) Error() string {
+	if err.msg != "" {
+		return fmt.Sprintf("Trust server rejected operation: %s", err.msg)
+	}
+	return "Trust server rejected operation."
+}
+
 // HTTPStore manages pulling and pushing metadata from and to a remote
 // service over HTTP. It assumes the URL structure of the remote service
 // maps identically to the structure of the TUF repo:
@@ -70,6 +83,19 @@ func NewHTTPStore(baseURL, metaPrefix, metaExtension, targetsPrefix, keyExtensio
 	}, nil
 }
 
+func translateStatusToError(resp *http.Response) error {
+	switch resp.StatusCode {
+	case http.StatusOK:
+		return nil
+	case http.StatusNotFound:
+		return ErrMetaNotFound{}
+	case http.StatusBadRequest:
+		return ErrInvalidOperation{}
+	default:
+		return ErrServerUnavailable{code: resp.StatusCode}
+	}
+}
+
 // GetMeta downloads the named meta file with the given size. A short body
 // is acceptable because in the case of timestamp.json, the size is a cap,
 // not an exact length.
@@ -87,11 +113,9 @@ func (s HTTPStore) GetMeta(name string, size int64) ([]byte, error) {
 		return nil, err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode == http.StatusNotFound {
+	if err := translateStatusToError(resp); err != nil {
-		return nil, ErrMetaNotFound{}
-	} else if resp.StatusCode != http.StatusOK {
 		logrus.Debugf("received HTTP status %d when requesting %s.", resp.StatusCode, name)
-		return nil, ErrServerUnavailable{code: resp.StatusCode}
+		return nil, err
 	}
 	if resp.ContentLength > size {
 		return nil, ErrMaliciousServer{}
@@ -120,12 +144,7 @@ func (s HTTPStore) SetMeta(name string, blob []byte) error {
 		return err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode == http.StatusNotFound {
+	return translateStatusToError(resp)
-		return ErrMetaNotFound{}
-	} else if resp.StatusCode != http.StatusOK {
-		return ErrServerUnavailable{code: resp.StatusCode}
-	}
-	return nil
 }
 
 // SetMultiMeta does a single batch upload of multiple pieces of TUF metadata.
@@ -159,12 +178,7 @@ func (s HTTPStore) SetMultiMeta(metas map[string][]byte) error {
 		return err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode == http.StatusNotFound {
+	return translateStatusToError(resp)
-		return ErrMetaNotFound{}
-	} else if resp.StatusCode != http.StatusOK {
-		return ErrServerUnavailable{code: resp.StatusCode}
-	}
-	return nil
 }
 
 func (s HTTPStore) buildMetaURL(name string) (*url.URL, error) {
@@ -212,10 +226,8 @@ func (s HTTPStore) GetTarget(path string) (io.ReadCloser, error) {
 		return nil, err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode == http.StatusNotFound {
+	if err := translateStatusToError(resp); err != nil {
-		return nil, ErrMetaNotFound{}
+		return nil, err
-	} else if resp.StatusCode != http.StatusOK {
-		return nil, ErrServerUnavailable{code: resp.StatusCode}
 	}
 	return resp.Body, nil
 }
@@ -235,10 +247,8 @@ func (s HTTPStore) GetKey(role string) ([]byte, error) {
 		return nil, err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode == http.StatusNotFound {
+	if err := translateStatusToError(resp); err != nil {
-		return nil, ErrMetaNotFound{}
+		return nil, err
-	} else if resp.StatusCode != http.StatusOK {
-		return nil, ErrServerUnavailable{code: resp.StatusCode}
 	}
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {

tuf/store/httpstore_test.go

@@ -178,11 +178,11 @@ func testErrorCode(t *testing.T, errorCode int, errType error) {
 		fmt.Sprintf("%d should translate to %v", errorCode, errType))
 }
 
-func TestErrMetadataNotFound(t *testing.T) {
+func Test404Error(t *testing.T) {
 	testErrorCode(t, http.StatusNotFound, ErrMetaNotFound{})
 }
 
-func Test500Errors(t *testing.T) {
+func Test50XErrors(t *testing.T) {
 	fiveHundreds := []int{
 		http.StatusInternalServerError,
 		http.StatusNotImplemented,
@@ -195,3 +195,7 @@ func Test500Errors(t *testing.T) {
 		testErrorCode(t, code, ErrServerUnavailable{})
 	}
 }
+
+func Test400Error(t *testing.T) {
+	testErrorCode(t, http.StatusBadRequest, ErrInvalidOperation{})
+}