WIP: Client rotate remote key

client/backwards_compatibility_test.go

@@ -107,12 +107,13 @@ func Test0Dot1RepoFormat(t *testing.T) {
 	// one and try to create a new one from scratch, which will be the wrong version
 	require.NoError(t, repo.fileStore.RemoveMeta(data.CanonicalTimestampRole))
 
-	// rotate the timestamp key, since the server doesn't have that one
+	// rotate the timestamp key, since the server doesn't have that one (we can't just
+	// call RotateKey because the server will verify that it is signed by the root, and
+	// the server doesn't know about this one)
 	timestampPubKey, err := getRemoteKey(ts.URL, gun, data.CanonicalTimestampRole, http.DefaultTransport)
 	require.NoError(t, err)
 	require.NoError(
 		t, repo.rootFileKeyChange(data.CanonicalTimestampRole, changelist.ActionCreate, timestampPubKey))
-
 	require.NoError(t, repo.Publish())
 
 	targets, err = repo.ListTargets()

client/client.go

@@ -55,6 +55,17 @@ func (err ErrInvalidRemoteRole) Error() string {
 		"notary does not support the server managing the %s key", err.Role)
 }
 
+// ErrInvalidLocalRole is returned when the client wants to manage
+// an unsupported key type
+type ErrInvalidLocalRole struct {
+	Role string
+}
+
+func (err ErrInvalidLocalRole) Error() string {
+	return fmt.Sprintf(
+		"notary does not support the client managing the %s key", err.Role)
+}
+
 // ErrRepositoryNotExist is returned when an action is taken on a remote
 // repository that doesn't exist
 type ErrRepositoryNotExist struct {
@@ -511,11 +522,10 @@ func (r *NotaryRepository) Publish() error {
 			initialPublish = true
 		} else {
 			// We could not update, so we cannot publish.
-			logrus.Error("Could not publish Repository: ", err.Error())
+			logrus.Error("Could not publish Repository since we could not update: ", err.Error())
 			return err
 		}
 	}
-
 	cl, err := r.GetChangelist()
 	if err != nil {
 		return err
@@ -608,7 +618,7 @@ func (r *NotaryRepository) Publish() error {
 // to load metadata for all roles.  Since server snapshots are supported,
 // if the snapshot metadata fails to load, that's ok.
 // This can also be unified with some cache reading tools from tuf/client.
-// This assumes that bootstrapRepo is only used by Publish()
+// This assumes that bootstrapRepo is only used by Publish() or RotateKey()
 func (r *NotaryRepository) bootstrapRepo() error {
 	kdb := keys.NewDB()
 	tufRepo := tuf.NewRepo(kdb, r.CryptoService)
@@ -829,28 +839,65 @@ func (r *NotaryRepository) validateRoot(rootJSON []byte) (*data.SignedRoot, erro
 // creates and adds one new key or delegates managing the key to the server.
 // These changes are staged in a changelist until publish is called.
 func (r *NotaryRepository) RotateKey(role string, serverManagesKey bool) error {
-	if role == data.CanonicalRootRole || role == data.CanonicalTimestampRole {
+	if role == data.CanonicalRootRole {
 		return fmt.Errorf(
 			"notary does not currently support rotating the %s key", role)
 	}
 	if serverManagesKey && role == data.CanonicalTargetsRole {
 		return ErrInvalidRemoteRole{Role: data.CanonicalTargetsRole}
 	}
+	if !serverManagesKey && role == data.CanonicalTimestampRole {
+		return ErrInvalidLocalRole{Role: data.CanonicalTimestampRole}
+	}
 
 	var (
-		pubKey data.PublicKey
-		err    error
+		pubKey       data.PublicKey
+		errFmtString string
+		err          error
 	)
 	if serverManagesKey {
-		pubKey, err = getRemoteKey(r.baseURL, r.gun, role, r.roundTrip)
+		if err := r.bootstrapRepo(); err != nil {
+			return err
+		}
+
+		// get root keys - since we bootstrapped the repo, a valid root should always specify
+		// the root role
+		rootRole, ok := r.tufRepo.Root.Signed.Roles[data.CanonicalRootRole]
+		if !ok {
+			return fmt.Errorf("update resulted in an invalid state with an invalid root")
+		}
+		rootKeys := make([]data.PublicKey, 0, len(rootRole.KeyIDs))
+		for _, kid := range rootRole.KeyIDs {
+			k, ok := r.tufRepo.Root.Signed.Keys[kid]
+			if ok {
+				// this should always be the case, as key IDs need to correspond to keys in the
+				// key list, but just in case: skip if it's not
+				rootKeys = append(rootKeys, k)
+			}
+		}
+
+		errFmtString = "unable to rotate remote key: %s"
+		remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip)
+		if err == nil {
+			pubKey, err = remote.RotateKey(role, r.CryptoService, rootKeys...)
+		}
 	} else {
 		pubKey, err = r.CryptoService.Create(role, data.ECDSAKey)
+		errFmtString = "unable to generate key: %s"
 	}
+
 	if err != nil {
+		return fmt.Errorf(errFmtString, err)
+	}
+
+	if err = r.rootFileKeyChange(role, changelist.ActionCreate, pubKey); err != nil {
 		return err
 	}
 
-	return r.rootFileKeyChange(role, changelist.ActionCreate, pubKey)
+	if serverManagesKey {
+		return r.Publish()
+	}
+	return nil
 }
 
 func (r *NotaryRepository) rootFileKeyChange(role, action string, key data.PublicKey) error {

client/client_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	ctxu "github.com/docker/distribution/context"
 	"github.com/docker/go/canonical/json"
+	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 
@@ -93,13 +94,13 @@ func (p *passRoleRecorder) assertAsked(t *testing.T, expected []string, args ...
 var passphraseRetriever = passphrase.ConstantRetriever(password)
 
 func simpleTestServer(t *testing.T, roles ...string) (
-	*httptest.Server, *http.ServeMux, map[string]data.PrivateKey) {
+	*httptest.Server, *mux.Router, map[string]data.PrivateKey) {
 
 	if len(roles) == 0 {
 		roles = []string{data.CanonicalTimestampRole, data.CanonicalSnapshotRole}
 	}
 	keys := make(map[string]data.PrivateKey)
-	mux := http.NewServeMux()
+	m := mux.NewRouter()
 
 	for _, role := range roles {
 		key, err := trustmanager.GenerateECDSAKey(rand.Reader)
@@ -112,15 +113,15 @@ func simpleTestServer(t *testing.T, roles ...string) (
 		keyJSON := string(jsonBytes)
 
 		// TUF will request /v2/docker.com/notary/_trust/tuf/<role>.key
-		mux.HandleFunc(
-			fmt.Sprintf("/v2/docker.com/notary/_trust/tuf/%s.key", role),
+		m.Methods("GET").Path(
+			fmt.Sprintf("/v2/docker.com/notary/_trust/tuf/%s.key", role)).HandlerFunc(
 			func(w http.ResponseWriter, r *http.Request) {
 				fmt.Fprint(w, keyJSON)
 			})
 	}
 
-	ts := httptest.NewServer(mux)
-	return ts, mux, keys
+	ts := httptest.NewServer(m)
+	return ts, m, keys
 }
 
 func fullTestServer(t *testing.T) *httptest.Server {
@@ -1019,7 +1020,7 @@ func testListEmptyTargets(t *testing.T, rootType string) {
 
 // reads data from the repository in order to fake data being served via
 // the ServeMux.
-func fakeServerData(t *testing.T, repo *NotaryRepository, mux *http.ServeMux,
+func fakeServerData(t *testing.T, repo *NotaryRepository, m *mux.Router,
 	keys map[string]data.PrivateKey) {
 
 	timestampKey, ok := keys[data.CanonicalTimestampRole]
@@ -1085,54 +1086,54 @@ func fakeServerData(t *testing.T, repo *NotaryRepository, mux *http.ServeMux,
 	cksmBytes = sha256.Sum256(level2JSON)
 	level2Checksum := hex.EncodeToString(cksmBytes[:])
 
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/root.json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/root.json",
 		func(w http.ResponseWriter, r *http.Request) {
 			assert.NoError(t, err)
 			fmt.Fprint(w, string(rootFileBytes))
 		})
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/root."+rootChecksum+".json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/root."+rootChecksum+".json",
 		func(w http.ResponseWriter, r *http.Request) {
 			assert.NoError(t, err)
 			fmt.Fprint(w, string(rootFileBytes))
 		})
 
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/timestamp.json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/timestamp.json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(timestampJSON))
 		})
 
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/snapshot.json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/snapshot.json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(snapshotJSON))
 		})
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/snapshot."+snapshotChecksum+".json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/snapshot."+snapshotChecksum+".json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(snapshotJSON))
 		})
 
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets.json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets.json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(targetsJSON))
 		})
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets."+targetsChecksum+".json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets."+targetsChecksum+".json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(targetsJSON))
 		})
 
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level1.json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level1.json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(level1JSON))
 		})
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level1."+level1Checksum+".json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level1."+level1Checksum+".json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(level1JSON))
 		})
 
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level2.json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level2.json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(level2JSON))
 		})
-	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level2."+level2Checksum+".json",
+	m.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level2."+level2Checksum+".json",
 		func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprint(w, string(level2JSON))
 		})
@@ -1146,7 +1147,7 @@ func (k targetSorter) Swap(i, j int)      { k[i], k[j] = k[j], k[i] }
 func (k targetSorter) Less(i, j int) bool { return k[i].Name < k[j].Name }
 
 func testListTarget(t *testing.T, rootType string) {
-	ts, mux, keys := simpleTestServer(t)
+	ts, m, keys := simpleTestServer(t)
 	defer ts.Close()
 
 	repo, _ := initializeRepo(t, rootType, "docker.com/notary", ts.URL, false)
@@ -1170,7 +1171,7 @@ func testListTarget(t *testing.T, rootType string) {
 	err = applyChangelist(repo.tufRepo, cl)
 	assert.NoError(t, err, "could not apply changelist")
 
-	fakeServerData(t, repo, mux, keys)
+	fakeServerData(t, repo, m, keys)
 
 	targets, err := repo.ListTargets(data.CanonicalTargetsRole)
 	assert.NoError(t, err)
@@ -1202,7 +1203,7 @@ func testListTarget(t *testing.T, rootType string) {
 }
 
 func testListTargetWithDelegates(t *testing.T, rootType string) {
-	ts, mux, keys := simpleTestServer(t)
+	ts, m, keys := simpleTestServer(t)
 	defer ts.Close()
 
 	repo, _ := initializeRepo(t, rootType, "docker.com/notary", ts.URL, false)
@@ -1253,7 +1254,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
 	_, ok = repo.tufRepo.Targets["targets/level2"].Signed.Targets["level2"]
 	assert.True(t, ok)
 
-	fakeServerData(t, repo, mux, keys)
+	fakeServerData(t, repo, m, keys)
 
 	// test default listing
 	targets, err := repo.ListTargets()
@@ -2392,14 +2393,16 @@ func TestRotateKeyInvalidRole(t *testing.T) {
 	repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false)
 	defer os.RemoveAll(repo.baseDir)
 
-	// the equivalent of: (root, true), (root, false), (timestamp, true),
-	// (timestamp, false), (targets, true)
+	// the equivalent of: (root, true), (root, false), (timestamp, false), (targets, true)
 	for _, role := range data.BaseRoles {
-		if role == data.CanonicalSnapshotRole {
+		if role == data.CanonicalSnapshotRole { // remote or local can manage snapshot
 			continue
 		}
 		for _, serverManagesKey := range []bool{true, false} {
-			if role == data.CanonicalTargetsRole && !serverManagesKey {
+			if role == data.CanonicalTargetsRole && !serverManagesKey { // only local can manage targets
+				continue
+			}
+			if role == data.CanonicalTimestampRole && serverManagesKey { // only remote can manage timestamp
 				continue
 			}
 			err := repo.RotateKey(role, serverManagesKey)
@@ -2567,6 +2570,39 @@ func testRotateKeySuccess(t *testing.T, serverManagesSnapshotInit bool,
 	}
 }
 
+// If remotely rotating key fails for a non-rate-limit reason, fail the rotation
+// entirely
+func TestRemoteRotationNonRateLimitError(t *testing.T) {
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, true)
+	defer os.RemoveAll(repo.baseDir)
+
+	// simpleTestServer has no rotate key endpoint, so this should fail
+	assert.Error(t, repo.RotateKey(data.CanonicalTimestampRole, true))
+}
+
+// The rotator is not the owner of the repository, they cannot rotate the remote
+// key
+func TestRemoteRotationNonPermitted(t *testing.T) {
+	ts := fullTestServer(t)
+	defer ts.Close()
+
+	repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, true)
+	defer os.RemoveAll(repo.baseDir)
+	assert.NoError(t, repo.Publish())
+
+	newRepo, _ := newRepoToTestRepo(t, repo, true)
+	defer os.RemoveAll(newRepo.baseDir)
+	_, err := newRepo.ListTargets()
+	assert.NoError(t, err)
+
+	err = newRepo.RotateKey(data.CanonicalSnapshotRole, true)
+	assert.Error(t, err)
+	assert.IsType(t, signed.ErrNoKeys{}, err)
+}
+
 // If there is no local cache, notary operations return the remote error code
 func TestRemoteServerUnavailableNoLocalCache(t *testing.T) {
 	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")

client/helpers.go

@@ -248,17 +248,7 @@ func getRemoteKey(url, gun, role string, rt http.RoundTripper) (data.PublicKey,
 	if err != nil {
 		return nil, err
 	}
-	rawPubKey, err := remote.GetKey(role)
-	if err != nil {
-		return nil, err
-	}
-
-	pubKey, err := data.UnmarshalPublicKey(rawPubKey)
-	if err != nil {
-		return nil, err
-	}
-
-	return pubKey, nil
+	return remote.GetKey(role)
 }
 
 // add a key to a KeyDB, and create a role for the key and add it.

cmd/notary/keys.go

@@ -37,7 +37,7 @@ var cmdKeyListTemplate = usageTemplate{
 var cmdRotateKeyTemplate = usageTemplate{
 	Use:   "rotate [ GUN ]",
 	Short: "Rotate the signing (non-root) keys for the given Globally Unique Name.",
-	Long:  "Removes all the old signing (non-root) keys for the given Globally Unique Name, and generates new ones.  This only makes local changes - please use then `notary publish` to push the key rotation changes to the remote server.",
+	Long:  "Removes old signing (non-root) keys for the given Globally Unique Name, and generates new ones.  If rotating to a server-managed key, the key rotation is automatically published.  If rotating to locally-managed key(s), only local, non-online changes are made - please use then `notary publish` to push the key rotation changes to the remote server.",
 }
 
 var cmdKeyGenerateRootKeyTemplate = usageTemplate{
@@ -361,13 +361,11 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error {
 		rolesToRotate = []string{data.CanonicalSnapshotRole}
 	case data.CanonicalTargetsRole:
 		rolesToRotate = []string{data.CanonicalTargetsRole}
+	case data.CanonicalTimestampRole:
+		rolesToRotate = []string{data.CanonicalTimestampRole}
 	default:
 		return fmt.Errorf("key rotation not supported for %s keys", k.rotateKeyRole)
 	}
-	if k.rotateKeyServerManaged && rotateKeyRole != data.CanonicalSnapshotRole {
-		return fmt.Errorf(
-			"remote signing/key management is only supported for the snapshot key")
-	}
 
 	config, err := k.configGetter()
 	if err != nil {