Adding tests to client, server, and httpstore. Update docs

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

client/client_test.go

@@ -2580,9 +2580,17 @@ func TestRotateKeyInvalidRole(t *testing.T) {
 	require.Error(t, repo.RotateKey("targets/releases", false),
 		"Rotating a delegation key should fail")
 
+	// rotating a delegation key to the server also fails
+	require.Error(t, repo.RotateKey("targets/releases", true),
+		"Rotating a delegation key should fail")
+
 	// rotating a not a real role key fails
 	require.Error(t, repo.RotateKey("nope", false),
 		"Rotating a non-real role key should fail")
+
+	// rotating a not a real role key to the server also fails
+	require.Error(t, repo.RotateKey("nope", false),
+		"Rotating a non-real role key should fail")
 }
 
 // If remotely rotating key fails, the failure is propagated
@@ -2595,9 +2603,59 @@ func TestRemoteRotationError(t *testing.T) {
 	ts.Close()
 
 	// server has died, so this should fail
+	for _, role := range []string{data.CanonicalSnapshotRole, data.CanonicalTimestampRole} {
+		err := repo.RotateKey(role, true)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "unable to rotate remote key")
+	}
+}
+
+// If remotely rotating key fails for any reason, fail the rotation entirely
+func TestRemoteRotationEndpointError(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
+	for _, role := range []string{data.CanonicalSnapshotRole, data.CanonicalTimestampRole} {
+		err := repo.RotateKey(role, true)
+		require.Error(t, err)
+		require.IsType(t, store.ErrMetaNotFound{}, err)
+	}
+}
+
+// The rotator is not the owner of the repository, they cannot rotate the remote
+// key
+func TestRemoteRotationNoRootKey(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)
+	require.NoError(t, repo.Publish())
+
+	newRepo, _ := newRepoToTestRepo(t, repo, true)
+	defer os.RemoveAll(newRepo.baseDir)
+	_, err := newRepo.ListTargets()
+	require.NoError(t, err)
+
+	err = newRepo.RotateKey(data.CanonicalSnapshotRole, true)
+	require.Error(t, err)
+	require.IsType(t, signed.ErrInsufficientSignatures{}, err)
+}
+
+// The repo hasn't been initialized, so we can't rotate
+func TestRemoteRotationNonexistentRepo(t *testing.T) {
+	ts, _, _ := simpleTestServer(t)
+	defer ts.Close()
+
+	repo := newBlankRepo(t, ts.URL)
+	defer os.RemoveAll(repo.baseDir)
+
 	err := repo.RotateKey(data.CanonicalTimestampRole, true)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "unable to rotate remote key")
+	require.IsType(t, ErrRepoNotInitialized{}, err)
 }
 
 // Rotates the keys.  After the rotation, downloading the latest metadata
@@ -2732,6 +2790,14 @@ func TestRotateKeyAfterPublishServerManagementChange(t *testing.T) {
 		data.CanonicalTargetsRole:  false,
 		data.CanonicalRootRole:     false,
 	})
+	// check that the snapshot remote rotation creates new keys
+	testRotateKeySuccess(t, false, map[string]bool{
+		data.CanonicalSnapshotRole: true,
+	})
+	// check that the timestamp remote rotation creates new keys
+	testRotateKeySuccess(t, false, map[string]bool{
+		data.CanonicalTimestampRole: true,
+	})
 	// reclaim snapshot key management from the server
 	testRotateKeySuccess(t, true, map[string]bool{
 		data.CanonicalSnapshotRole: false,

docs/advanced_usage.md

@@ -87,7 +87,7 @@ subsection.
 
 ### Rotate keys
 
-In case of potential compromise, notary provides a CLI command for rotating keys. Currently, you can use the `notary key rotate` command to rotate the targets or snapshot keys.
+In case of potential compromise, notary provides a CLI command for rotating keys. Currently, you can use the `notary key rotate` command to rotate the root, targets, snapshot, or timestamp keys.
 
 While the snapshot key is managed by the notary client by default, use the `notary key
 rotate snapshot -r` command to rotate the snapshot key to the server, such that the
@@ -98,7 +98,8 @@ snapshot key to push their updates to the collection.
 Note that new collections created by a Docker 1.11 Engine client will have the server manage the snapshot key by default.
 To reclaim control of the snapshot key on the client, use the `notary key rotate` command without the `-r` flag.
 
-The targets key must be locally managed - to rotate the targets key, for instance in case of compromise, use the `notary key rotate targets` command without the `-r` flag.
+The root and targets key must be locally managed - to rotate either the root or targets key, for instance in case of compromise, use the `notary key rotate` command without the `-r` flag.
+The timestamp key must be remotely managed - to rotate the timestamp key use the `notary key rotate <GUN> timestamp -r` command.
 
 ### Use a Yubikey
 

server/errors/errors.go

@@ -92,11 +92,5 @@ var (
 		Description:    "The server does not support actions on images of this name",
 		HTTPStatusCode: http.StatusBadRequest,
 	})
-	ErrKeyRotationLimited = errcode.Register(errGroup, errcode.ErrorDescriptor{
-		Value:          "ROTATE_KEY_RATE_LIMITED",
-		Message:        "Cannot rotate, last key rotation too recent.",
-		Description:    "Cannot rotate key because the last key rotation was too recent",
-		HTTPStatusCode: http.StatusTooManyRequests,
-	})
 	ErrUnknown = errcode.ErrorCodeUnknown
 )

server/handlers/default_test.go

@@ -76,9 +76,11 @@ func TestMainHandlerNotGet(t *testing.T) {
 	}
 }
 
-// GetKeyHandler needs to have access to a metadata store and cryptoservice,
+type simplerHandler func(context.Context, http.ResponseWriter, *http.Request, map[string]string) error
+
+// GetKeyHandler and RotateKeyHandler needs to have access to a metadata store and cryptoservice,
 // a key algorithm
-func TestGetKeyHandlerInvalidConfiguration(t *testing.T) {
+func TestKeyHandlersInvalidConfiguration(t *testing.T) {
 	noStore := defaultState()
 	noStore.store = nil
 
@@ -108,38 +110,41 @@ func TestGetKeyHandlerInvalidConfiguration(t *testing.T) {
 		"tufRole":   data.CanonicalTimestampRole,
 	}
 	req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))}
-	for errString, states := range invalidStates {
-		for _, s := range states {
-			err := getKeyHandler(getContext(s), httptest.NewRecorder(), req, vars)
-			require.Error(t, err)
-			require.Contains(t, err.Error(), errString)
+	for _, keyHandler := range []simplerHandler{getKeyHandler, rotateKeyHandler} {
+		for errString, states := range invalidStates {
+			for _, s := range states {
+				err := keyHandler(getContext(s), httptest.NewRecorder(), req, vars)
+				require.Error(t, err)
+				require.Contains(t, err.Error(), errString)
+			}
 		}
 	}
 }
 
-// GetKeyHandler needs to be set up such that an imageName and tufRole are both
+// GetKeyHandler and RotateKeyHandler need to be set up such that an imageName and tufRole are both
 // provided and non-empty.
-func TestGetKeyHandlerNoRoleOrRepo(t *testing.T) {
+func TestKeyHandlersNoRoleOrRepo(t *testing.T) {
 	state := defaultState()
 	req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))}
+	for _, keyHandler := range []simplerHandler{getKeyHandler, rotateKeyHandler} {
+		for _, key := range []string{"imageName", "tufRole"} {
+			vars := map[string]string{
+				"imageName": "gun",
+				"tufRole":   data.CanonicalTimestampRole,
+			}
+
+			// not provided
+			delete(vars, key)
+			err := keyHandler(getContext(state), httptest.NewRecorder(), req, vars)
+			require.Error(t, err)
+			require.Contains(t, err.Error(), "unknown")
 
-	for _, key := range []string{"imageName", "tufRole"} {
-		vars := map[string]string{
-			"imageName": "gun",
-			"tufRole":   data.CanonicalTimestampRole,
+			// empty
+			vars[key] = ""
+			err = keyHandler(getContext(state), httptest.NewRecorder(), req, vars)
+			require.Error(t, err)
+			require.Contains(t, err.Error(), "unknown")
 		}
-
-		// not provided
-		delete(vars, key)
-		err := getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars)
-		require.Error(t, err)
-		require.Contains(t, err.Error(), "unknown")
-
-		// empty
-		vars[key] = ""
-		err = getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars)
-		require.Error(t, err)
-		require.Contains(t, err.Error(), "unknown")
 	}
 }
 
@@ -172,6 +177,55 @@ func TestGetKeyHandlerCreatesOnce(t *testing.T) {
 	}
 }
 
+// Rotating a key for a non-supported role results in a 400.
+func TestRotateKeyHandlerInvalidRole(t *testing.T) {
+	state := defaultState()
+	for _, role := range []string{data.CanonicalRootRole, data.CanonicalTargetsRole, "targets/a", "invalidrole"} {
+		vars := map[string]string{
+			"imageName": "gun",
+			"tufRole":   role,
+		}
+		req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))}
+
+		err := rotateKeyHandler(getContext(state), httptest.NewRecorder(), req, vars)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "invalid role")
+	}
+}
+
+// Rotating the key fails if we don't pass a valid key algorithm, or if it isn't Ed25519
+func TestRotateKeyHandlerNonED25519(t *testing.T) {
+	roles := []string{data.CanonicalTimestampRole, data.CanonicalSnapshotRole}
+	req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))}
+
+	for _, role := range roles {
+		vars := map[string]string{"imageName": "gun", "tufRole": role}
+		recorder := httptest.NewRecorder()
+		invalidKeyAlgoState := defaultState()
+		invalidKeyAlgoState.keyAlgo = "notactuallyakeyalgorithm"
+		err := rotateKeyHandler(getContext(invalidKeyAlgoState), recorder, req, vars)
+		require.Error(t, err)
+		invalidKeyAlgoState.keyAlgo = data.ECDSAKey
+		err = rotateKeyHandler(getContext(invalidKeyAlgoState), recorder, req, vars)
+		require.Error(t, err)
+	}
+}
+
+// Rotating the key for a valid role and gun succeeds
+func TestRotateKeyHandlerSuccessfulRotation(t *testing.T) {
+	state := defaultState()
+	roles := []string{data.CanonicalTimestampRole, data.CanonicalSnapshotRole}
+	req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))}
+
+	for _, role := range roles {
+		vars := map[string]string{"imageName": "gun", "tufRole": role}
+		recorder := httptest.NewRecorder()
+		err := rotateKeyHandler(getContext(state), recorder, req, vars)
+		require.NoError(t, err)
+		require.True(t, len(recorder.Body.String()) > 0)
+	}
+}
+
 func TestGetHandlerRoot(t *testing.T) {
 	metaStore := storage.NewMemStorage()
 	repo, _, err := testutils.EmptyRepo("gun")

server/server_test.go

@@ -304,3 +304,32 @@ func verifyGetResponse(t *testing.T, r *http.Response, expectedBytes []byte) {
 	require.NotEqual(t, "", r.Header.Get("Last-Modified"))
 	require.Equal(t, "", r.Header.Get("Pragma"))
 }
+
+// RotateKey supports only timestamp and snapshot key rotation
+func TestRotateKeyEndpoint(t *testing.T) {
+	ctx := context.WithValue(
+		context.Background(), "metaStore", storage.NewMemStorage())
+	ctx = context.WithValue(ctx, "keyAlgorithm", data.ED25519Key)
+
+	ccc := utils.NewCacheControlConfig(10, false)
+	handler := RootHandler(nil, ctx, signed.NewEd25519(), ccc, ccc, nil)
+	ts := httptest.NewServer(handler)
+	defer ts.Close()
+
+	rolesToStatus := map[string]int{
+		data.CanonicalTimestampRole: http.StatusOK, // just returning same as GetKey endpoint right now
+		data.CanonicalSnapshotRole:  http.StatusOK, // just returning same as GetKey endpoint right now
+		data.CanonicalTargetsRole:   http.StatusNotFound,
+		data.CanonicalRootRole:      http.StatusNotFound,
+		"targets/delegation":        http.StatusNotFound,
+		"somerandomrole":            http.StatusNotFound,
+	}
+	var buf bytes.Buffer
+	for role, expectedStatus := range rolesToStatus {
+		res, err := http.Post(
+			fmt.Sprintf("%s/v2/gun/_trust/tuf/%s.key", ts.URL, role),
+			"text/plain", &buf)
+		require.NoError(t, err)
+		require.Equal(t, expectedStatus, res.StatusCode)
+	}
+}

signer/keydbstore/cachedkeystore_test.go

@@ -74,7 +74,7 @@ func TestGetSuccessPopulatesCache(t *testing.T) {
 	requireGetKeySuccessFromCache(t, cached, underlying, data.CanonicalTimestampRole, testKey)
 }
 
-// Creating a key, on succcess, populates the cache, but does not do so on failure
+// Creating a key, on success, populates the cache, but does not do so on failure
 func TestAddKeyPopulatesCacheIfSuccessful(t *testing.T) {
 	var underlying trustmanager.KeyStore
 	underlying = trustmanager.NewKeyMemoryStore(constRetriever)

signer/keydbstore/keydbstore_test.go

@@ -36,6 +36,7 @@ type keyRotator interface {
 type keyActivator interface {
 	trustmanager.KeyStore
 	MarkActive(keyID string) error
+	GetPendingKey(trustmanager.KeyInfo) (data.PublicKey, error)
 }
 
 // A key can only be added to the DB once.  Returns a list of expected keys, and which keys are expected to exist.
@@ -139,3 +140,37 @@ func testMarkKeyActive(t *testing.T, dbStore keyActivator) (data.PrivateKey, dat
 
 	return testKeys[0], testKeys[1]
 }
+
+func testGetPendingKey(t *testing.T, dbStore keyActivator) (data.PrivateKey, data.PrivateKey) {
+	// Create a test key and add it to the db such that it will be pending (never marked active)
+	keyInfo := trustmanager.KeyInfo{Role: data.CanonicalSnapshotRole, Gun: "gun"}
+	pendingTestKey, err := utils.GenerateECDSAKey(rand.Reader)
+	require.NoError(t, err)
+	requireGetKeyFailure(t, dbStore, pendingTestKey.ID())
+	err = dbStore.AddKey(keyInfo, pendingTestKey)
+	require.NoError(t, err)
+	requireGetKeySuccess(t, dbStore, data.CanonicalSnapshotRole, pendingTestKey)
+
+	retrievedKey, err := dbStore.GetPendingKey(keyInfo)
+	require.NoError(t, err)
+	require.Equal(t, pendingTestKey.Public(), retrievedKey.Public())
+
+	// Now create an active key with the same keyInfo
+	activeTestKey, err := utils.GenerateECDSAKey(rand.Reader)
+	require.NoError(t, err)
+	requireGetKeyFailure(t, dbStore, activeTestKey.ID())
+	err = dbStore.AddKey(keyInfo, activeTestKey)
+	require.NoError(t, err)
+	requireGetKeySuccess(t, dbStore, data.CanonicalSnapshotRole, activeTestKey)
+
+	// Mark as active
+	require.NoError(t, dbStore.MarkActive(activeTestKey.ID()))
+
+	// We should still get back the original pending key on GetPendingKey
+	retrievedKey, err = dbStore.GetPendingKey(keyInfo)
+	require.NoError(t, err)
+	require.NotEqual(t, activeTestKey.Public(), retrievedKey.Public())
+	require.Equal(t, pendingTestKey.Public(), retrievedKey.Public())
+
+	return pendingTestKey, activeTestKey
+}

signer/keydbstore/rethink_realkeydbstore_test.go

@@ -209,3 +209,16 @@ func TestRethinkMarkKeyActive(t *testing.T) {
 	require.True(t, rdbKeys[activeKey.ID()].LastUsed.Equal(rdbNow))
 	require.True(t, rdbKeys[nonActiveKey.ID()].LastUsed.Equal(time.Time{}))
 }
+
+func TestRethinkGetPendingKey(t *testing.T) {
+	dbStore, cleanup := rethinkDBSetup(t, "signerActivationTests")
+	defer cleanup()
+
+	pendingKey, activeKey := testGetPendingKey(t, dbStore)
+
+	rdbKeys := requireExpectedRDBKeys(t, dbStore, []data.PrivateKey{pendingKey, activeKey})
+
+	// check that activation updates the activated key but not the unactivated key
+	require.True(t, rdbKeys[activeKey.ID()].LastUsed.Equal(rdbNow))
+	require.True(t, rdbKeys[pendingKey.ID()].LastUsed.Equal(time.Time{}))
+}

signer/keydbstore/sql_keydbstore_test.go

@@ -157,3 +157,16 @@ func TestSQLMarkKeyActive(t *testing.T) {
 	require.True(t, gormKeys[activeKey.ID()].LastUsed.Equal(gormActiveTime))
 	require.True(t, gormKeys[nonActiveKey.ID()].LastUsed.Equal(time.Time{}))
 }
+
+func TestSQLGetPendingKey(t *testing.T) {
+	dbStore, cleanup := sqldbSetup(t)
+	defer cleanup()
+
+	pendingKey, activeKey := testGetPendingKey(t, dbStore)
+
+	gormKeys := requireExpectedGORMKeys(t, dbStore, []data.PrivateKey{pendingKey, activeKey})
+
+	// check that activation updates the activated key but not the pending key
+	require.True(t, gormKeys[activeKey.ID()].LastUsed.Equal(gormActiveTime))
+	require.True(t, gormKeys[pendingKey.ID()].LastUsed.Equal(time.Time{}))
+}