diff --git a/client/backwards_compatibility_test.go b/client/backwards_compatibility_test.go index d78f313f9..833c2aec4 100644 --- a/client/backwards_compatibility_test.go +++ b/client/backwards_compatibility_test.go @@ -12,7 +12,6 @@ import ( "testing" "time" - "github.com/docker/notary/client/changelist" "github.com/docker/notary/passphrase" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/store" @@ -107,13 +106,9 @@ 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 - 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()) + // We need to rotate the timestamp key, since the server doesn't have the one + // specified by the metadata. + require.NoError(t, repo.RotateKey(data.CanonicalTimestampRole, true)) targets, err = repo.ListTargets() require.NoError(t, err) diff --git a/client/client.go b/client/client.go index 444063d00..b2c0540df 100644 --- a/client/client.go +++ b/client/client.go @@ -52,7 +52,18 @@ type ErrInvalidRemoteRole struct { func (err ErrInvalidRemoteRole) Error() string { return fmt.Sprintf( - "notary does not support the server managing the %s key", err.Role) + "notary does not permit 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 permit the client managing the %s key", err.Role) } // ErrRepositoryNotExist is returned when an action is taken on a remote @@ -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,68 @@ 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 { + switch { + case role == data.CanonicalRootRole: return fmt.Errorf( - "notary does not currently support rotating the %s key", role) - } - if serverManagesKey && role == data.CanonicalTargetsRole { + "notary does not currently permit rotating the %s key", role) + + // We currently support locally managing targets keys, remotely managing + // timestamp keys, and locally or remotely managing snapshot keys. + case serverManagesKey && role == data.CanonicalTargetsRole: return ErrInvalidRemoteRole{Role: data.CanonicalTargetsRole} + case !serverManagesKey && role == data.CanonicalTimestampRole: + return ErrInvalidLocalRole{Role: data.CanonicalTimestampRole} } var ( - pubKey data.PublicKey - err error + pubKey data.PublicKey + errFmtString string + err error ) + // Key rotation is an offline operation unless we are rotating a remote key, in which case we + // need to ensure the rotation can actually happen: the user needs to have the root key, and + // we want to publish right away so that the remote key doesn't expire if serverManagesKey { - pubKey, err = getRemoteKey(r.baseURL, r.gun, role, r.roundTrip) + if err = r.bootstrapRepo(); err != nil { + if _, ok := err.(store.ErrMetaNotFound); ok { + err = ErrRepoNotInitialized{} + } + logrus.Debug("Repository should be valid and initialized before rotating a remote key:", err) + return err + } + + // get root keys - since we bootstrapped the repo, a valid root should always specify + // the root role + rootKeys, err := r.tufRepo.SigningKeysForRole(data.CanonicalRootRole) + if err != nil { + logrus.Debug("The root is invalid, because it does not contain any signing keys") + return err + } + + 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 if _, ok := err.(signed.ErrNoKeys); ok { + err = fmt.Errorf("root signing key unavailable so unable to rotate key") + } } 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 { diff --git a/client/client_test.go b/client/client_test.go index 1803dd6b6..1fcecc8b4 100644 --- a/client/client_test.go +++ b/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/.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 { @@ -247,7 +248,7 @@ func TestInitRepositoryManagedRolesIncludingRoot(t *testing.T) { assert.IsType(t, ErrInvalidRemoteRole{}, err) // Just testing the error message here in this one case assert.Equal(t, err.Error(), - "notary does not support the server managing the root key") + "notary does not permit the server managing the root key") // no key creation happened rec.assertCreated(t, nil) } @@ -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,54 @@ 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 + err := repo.RotateKey(data.CanonicalTimestampRole, true) + assert.Error(t, err) + assert.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) + 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) +} + +// 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) + assert.Error(t, err) + assert.IsType(t, ErrRepoNotInitialized{}, 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-") diff --git a/client/helpers.go b/client/helpers.go index 41b0b686c..bee7147b8 100644 --- a/client/helpers.go +++ b/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. diff --git a/cmd/notary/keys.go b/cmd/notary/keys.go index 353ec750c..5ddfdd3b3 100644 --- a/cmd/notary/keys.go +++ b/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 { diff --git a/cmd/notary/keys_test.go b/cmd/notary/keys_test.go index 95bf0979c..9d989caa5 100644 --- a/cmd/notary/keys_test.go +++ b/cmd/notary/keys_test.go @@ -11,10 +11,16 @@ import ( "strings" "testing" - "github.com/docker/go/canonical/json" + "golang.org/x/net/context" + + "github.com/Sirupsen/logrus" + ctxu "github.com/docker/distribution/context" "github.com/docker/notary" "github.com/docker/notary/client" + "github.com/docker/notary/cryptoservice" "github.com/docker/notary/passphrase" + "github.com/docker/notary/server" + "github.com/docker/notary/server/storage" "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" "github.com/spf13/cobra" @@ -227,12 +233,12 @@ func TestRemoveMultikeysRemoveOnlyChosenKey(t *testing.T) { } } -// Non-roles, root, and timestamp can't be rotated +// Non-roles, root, and delegation keys can't be rotated with this command line func TestRotateKeyInvalidRoles(t *testing.T) { invalids := []string{ data.CanonicalRootRole, - data.CanonicalTimestampRole, "notevenARole", + "targets/a", } for _, role := range invalids { for _, serverManaged := range []bool{true, false} { @@ -260,8 +266,20 @@ func TestRotateKeyTargetCannotBeServerManaged(t *testing.T) { } err := k.keysRotate(&cobra.Command{}, []string{"gun"}) assert.Error(t, err) - assert.Contains(t, err.Error(), - "remote signing/key management is only supported for the snapshot key") + assert.IsType(t, client.ErrInvalidRemoteRole{}, err) +} + +// Cannot rotate a timestamp key and require that it is locally managed it +func TestRotateKeyTimestampCannotBeLocallyManaged(t *testing.T) { + k := &keyCommander{ + configGetter: func() (*viper.Viper, error) { return viper.New(), nil }, + getRetriever: func() passphrase.Retriever { return passphrase.ConstantRetriever("pass") }, + rotateKeyRole: data.CanonicalTimestampRole, + rotateKeyServerManaged: false, + } + err := k.keysRotate(&cobra.Command{}, []string{"gun"}) + assert.Error(t, err) + assert.IsType(t, client.ErrInvalidLocalRole{}, err) } // rotate key must be provided with a gun @@ -280,17 +298,23 @@ func TestRotateKeyNoGUN(t *testing.T) { func setUpRepo(t *testing.T, tempBaseDir, gun string, ret passphrase.Retriever) ( *httptest.Server, map[string]string) { - // server that always returns 200 (and a key) - key, err := trustmanager.GenerateECDSAKey(rand.Reader) - assert.NoError(t, err) - pubKey := data.PublicKeyFromPrivate(key) - jsonBytes, err := json.MarshalCanonical(&pubKey) - assert.NoError(t, err) - keyJSON := string(jsonBytes) - ts := httptest.NewServer(http.HandlerFunc( - func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, keyJSON) - })) + // Set up server + ctx := context.WithValue( + context.Background(), "metaStore", storage.NewMemStorage()) + + // Do not pass one of the const KeyAlgorithms here as the value! Passing a + // string is in itself good test that we are handling it correctly as we + // will be receiving a string from the configuration. + ctx = context.WithValue(ctx, "keyAlgorithm", "ecdsa") + + // Eat the logs instead of spewing them out + l := logrus.New() + l.Out = bytes.NewBuffer(nil) + ctx = ctxu.WithLogger(ctx, logrus.NewEntry(l)) + + cryptoService := cryptoservice.NewCryptoService( + "", trustmanager.NewKeyMemoryStore(ret)) + ts := httptest.NewServer(server.RootHandler(nil, ctx, cryptoService)) repo, err := client.NewNotaryRepository( tempBaseDir, gun, ts.URL, http.DefaultTransport, ret) @@ -309,39 +333,59 @@ func setUpRepo(t *testing.T, tempBaseDir, gun string, ret passphrase.Retriever) // that the correct config variables are passed for the client to request a key // from the remote server. func TestRotateKeyRemoteServerManagesKey(t *testing.T) { - // Temporary directory where test files will be created - tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") - defer os.RemoveAll(tempBaseDir) - assert.NoError(t, err, "failed to create a temporary directory: %s", err) - gun := "docker.com/notary" - - ret := passphrase.ConstantRetriever("pass") + for _, role := range []string{data.CanonicalSnapshotRole, data.CanonicalTimestampRole} { + // Temporary directory where test files will be created + tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-") + defer os.RemoveAll(tempBaseDir) + assert.NoError(t, err, "failed to create a temporary directory: %s", err) + gun := "docker.com/notary" + + ret := passphrase.ConstantRetriever("pass") + + ts, initialKeys := setUpRepo(t, tempBaseDir, gun, ret) + defer ts.Close() + + k := &keyCommander{ + configGetter: func() (*viper.Viper, error) { + v := viper.New() + v.SetDefault("trust_dir", tempBaseDir) + v.SetDefault("remote_server.url", ts.URL) + return v, nil + }, + getRetriever: func() passphrase.Retriever { return ret }, + rotateKeyRole: role, + rotateKeyServerManaged: true, + } + err = k.keysRotate(&cobra.Command{}, []string{gun}) + assert.NoError(t, err) - ts, initialKeys := setUpRepo(t, tempBaseDir, gun, ret) - defer ts.Close() + repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, nil, ret) + assert.NoError(t, err, "error creating repo: %s", err) + + cl, err := repo.GetChangelist() + assert.NoError(t, err, "unable to get changelist: %v", err) + assert.Len(t, cl.List(), 0, "The key rotation should have been published") + + finalKeys := repo.CryptoService.ListAllKeys() + assert.Len(t, initialKeys, 3) + // no keys have been created, since a remote key was specified + if role == data.CanonicalSnapshotRole { + assert.Len(t, finalKeys, 2) + for k, r := range initialKeys { + if r != data.CanonicalSnapshotRole { + _, ok := finalKeys[k] + assert.True(t, ok) + } + } + } else { + assert.Len(t, finalKeys, 3) + for k := range initialKeys { + _, ok := finalKeys[k] + assert.True(t, ok) + } + } - k := &keyCommander{ - configGetter: func() (*viper.Viper, error) { - v := viper.New() - v.SetDefault("trust_dir", tempBaseDir) - v.SetDefault("remote_server.url", ts.URL) - return v, nil - }, - getRetriever: func() passphrase.Retriever { return ret }, - rotateKeyRole: data.CanonicalSnapshotRole, - rotateKeyServerManaged: true, } - err = k.keysRotate(&cobra.Command{}, []string{gun}) - assert.NoError(t, err) - - repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, nil, ret) - assert.NoError(t, err, "error creating repo: %s", err) - - cl, err := repo.GetChangelist() - assert.NoError(t, err, "unable to get changelist: %v", err) - assert.Len(t, cl.List(), 1) - // no keys have been created, since a remote key was specified - assert.Equal(t, initialKeys, repo.CryptoService.ListAllKeys()) } // The command line uses NotaryRepository's RotateKey - this is just testing diff --git a/const.go b/const.go index 3841bc8c8..b1c917554 100644 --- a/const.go +++ b/const.go @@ -18,4 +18,7 @@ const ( Sha256HexSize = 64 // TrustedCertsDir is the directory, under the notary repo base directory, where trusted certs are stored TrustedCertsDir = "trusted_certificates" + + // HTTPStatusTooManyRequests is the http status 429 - will be an exported constant in http library in Go 1.6 + HTTPStatusTooManyRequests = 429 ) diff --git a/server/errors/errors.go b/server/errors/errors.go index ad92ccf89..714066289 100644 --- a/server/errors/errors.go +++ b/server/errors/errors.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/docker/distribution/registry/api/errcode" + "github.com/docker/notary" ) // The notary API is on version 1, but URLs start with /v2/ to be consistent @@ -81,10 +82,16 @@ var ( HTTPStatusCode: http.StatusInternalServerError, }) ErrNoKeyAlgorithm = errcode.Register(errGroup, errcode.ErrorDescriptor{ - Value: "NO_KEYALGORITHM", + Value: "NO_KEY_ALGORITHM", Message: "The server does not have a key algorithm configured.", - Description: "No key algorihtm has been configured for the server and it has been asked to perform an operation that requires generation.", + Description: "No key algorithm has been configured for the server and it has been asked to perform an operation that requires generation.", HTTPStatusCode: http.StatusInternalServerError, }) + 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: notary.HTTPStatusTooManyRequests, + }) ErrUnknown = errcode.ErrorCodeUnknown ) diff --git a/server/handlers/default.go b/server/handlers/default.go index 96a601412..0ed726df0 100644 --- a/server/handlers/default.go +++ b/server/handlers/default.go @@ -143,70 +143,94 @@ func DeleteHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) return nil } +// RotateKeyHandler returns a new public key for the specified role, creating a new key-pair +// if one has not be rotated yet within a certain time period. +func RotateKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) error { + defer r.Body.Close() + vars := mux.Vars(r) + return rotateKeyHandler(ctx, w, vars) +} + // GetKeyHandler returns a public key for the specified role, creating a new key-pair // it if it doesn't yet exist func GetKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) error { defer r.Body.Close() vars := mux.Vars(r) - return getKeyHandler(ctx, w, r, vars) + return getKeyHandler(ctx, w, vars) +} + +type serverKeyInfo struct { + gun string + role string + store storage.MetaStore + crypto signed.CryptoService + keyAlgo string } -func getKeyHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { +func parseKeyParams(ctx context.Context, vars map[string]string) (*serverKeyInfo, error) { gun, ok := vars["imageName"] if !ok || gun == "" { - return errors.ErrUnknown.WithDetail("no gun") + return nil, errors.ErrUnknown.WithDetail("no gun") } role, ok := vars["tufRole"] if !ok || role == "" { - return errors.ErrUnknown.WithDetail("no role") + return nil, errors.ErrUnknown.WithDetail("no role") } - logger := ctxu.GetLoggerWithField(ctx, gun, "gun") - s := ctx.Value("metaStore") store, ok := s.(storage.MetaStore) if !ok || store == nil { - logger.Error("500 GET storage not configured") - return errors.ErrNoStorage.WithDetail(nil) + return nil, errors.ErrNoStorage.WithDetail("metadata store not configured") } + c := ctx.Value("cryptoService") crypto, ok := c.(signed.CryptoService) if !ok || crypto == nil { - logger.Error("500 GET crypto service not configured") - return errors.ErrNoCryptoService.WithDetail(nil) + return nil, errors.ErrNoCryptoService.WithDetail("crypto service not configured") } + algo := ctx.Value("keyAlgorithm") keyAlgo, ok := algo.(string) if !ok || keyAlgo == "" { - logger.Error("500 GET key algorithm not configured") - return errors.ErrNoKeyAlgorithm.WithDetail(nil) + return nil, errors.ErrNoKeyAlgorithm.WithDetail("key algorithm not configured") + } + + return &serverKeyInfo{ + gun: gun, + role: role, + store: store, + crypto: crypto, + keyAlgo: keyAlgo, + }, nil +} + +func rotateKeyHandler(ctx context.Context, w io.Writer, vars map[string]string) error { + return getKeyHandler(ctx, w, vars) +} + +func getKeyHandler(ctx context.Context, w io.Writer, vars map[string]string) error { + s, err := parseKeyParams(ctx, vars) + if err != nil { + return err } - keyAlgorithm := keyAlgo + var key data.PublicKey - var ( - key data.PublicKey - err error - ) - switch role { + switch s.role { // parseKeyParams ensures it's only timestamp or snapshot case data.CanonicalTimestampRole: - key, err = timestamp.GetOrCreateTimestampKey(gun, store, crypto, keyAlgorithm) + key, err = timestamp.GetOrCreateTimestampKey(s.gun, s.store, s.crypto, s.keyAlgo) case data.CanonicalSnapshotRole: - key, err = snapshot.GetOrCreateSnapshotKey(gun, store, crypto, keyAlgorithm) + key, err = snapshot.GetOrCreateSnapshotKey(s.gun, s.store, s.crypto, s.keyAlgo) default: - logger.Errorf("400 GET %s key: %v", role, err) - return errors.ErrInvalidRole.WithDetail(role) + return errors.ErrInvalidRole.WithDetail(s.role) } if err != nil { - logger.Errorf("500 GET %s key: %v", role, err) return errors.ErrUnknown.WithDetail(err) } out, err := json.Marshal(key) if err != nil { - logger.Errorf("500 GET %s key", role) return errors.ErrUnknown.WithDetail(err) } - logger.Debugf("200 GET %s key", role) w.Write(out) return nil } diff --git a/server/handlers/default_test.go b/server/handlers/default_test.go index 4669b90e8..8dda4b938 100644 --- a/server/handlers/default_test.go +++ b/server/handlers/default_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "io" "io/ioutil" "net/http" "net/http/httptest" @@ -19,10 +20,10 @@ import ( "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/store" "github.com/docker/notary/tuf/validation" + "github.com/stretchr/testify/assert" "github.com/docker/notary/tuf/testutils" "github.com/docker/notary/utils" - "github.com/stretchr/testify/assert" ) type handlerState struct { @@ -75,9 +76,11 @@ func TestMainHandlerNotGet(t *testing.T) { } } -// GetKeyHandler needs to have access to a metadata store and cryptoservice, -// a key algorithm -func TestGetKeyHandlerInvalidConfiguration(t *testing.T) { +type simplerHandler func(context.Context, io.Writer, map[string]string) error + +// GetKeyHandler and RotateKeyHandler needs to have access to a metadata store +// and cryptoservice, a key algorithm +func TestKeyHandlersInvalidConfiguration(t *testing.T) { noStore := defaultState() noStore.store = nil @@ -99,75 +102,80 @@ func TestGetKeyHandlerInvalidConfiguration(t *testing.T) { invalidStates := map[string][]handlerState{ "no storage": {noStore, invalidStore}, "no cryptoservice": {noCrypto, invalidCrypto}, - "no keyalgorithm": {noKeyAlgo, invalidKeyAlgo}, + "no key algorithm": {noKeyAlgo, invalidKeyAlgo}, } vars := map[string]string{ "imageName": "gun", "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) - assert.Error(t, err) - assert.Contains(t, err.Error(), errString) + var buf bytes.Buffer + for _, keyHandler := range []simplerHandler{getKeyHandler, rotateKeyHandler} { + for errString, states := range invalidStates { + for _, s := range states { + err := keyHandler(getContext(s), &buf, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), errString) + } } } } -// GetKeyHandler needs to be set up such that an imageName and tufRole are both -// provided and non-empty. -func TestGetKeyHandlerNoRoleOrRepo(t *testing.T) { +// GetKeyHandler and RotateKeyHandler needs to be set up such that an imageName +// and tufRole are both provided and non-empty. +func TestKeyHandlersNoRoleOrRepo(t *testing.T) { state := defaultState() - req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} - for _, key := range []string{"imageName", "tufRole"} { - vars := map[string]string{ - "imageName": "gun", - "tufRole": data.CanonicalTimestampRole, - } - - // not provided - delete(vars, key) - err := getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars) - assert.Error(t, err) - assert.Contains(t, err.Error(), "unknown") + for _, keyHandler := range []simplerHandler{getKeyHandler, rotateKeyHandler} { + for _, key := range []string{"imageName", "tufRole"} { + vars := map[string]string{ + "imageName": "gun", + "tufRole": data.CanonicalTimestampRole, + } + var buf bytes.Buffer + + // not provided + delete(vars, key) + err := keyHandler(getContext(state), &buf, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unknown") - // empty - vars[key] = "" - err = getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars) - assert.Error(t, err) - assert.Contains(t, err.Error(), "unknown") + // empty + vars[key] = "" + err = keyHandler(getContext(state), &buf, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unknown") + } } } -// Getting a key for a non-supported role results in a 400. -func TestGetKeyHandlerInvalidRole(t *testing.T) { +// Getting or rotating a key for a non-supported role results in a 400. +func TestKeyHandlersInvalidRole(t *testing.T) { state := defaultState() vars := map[string]string{ "imageName": "gun", "tufRole": data.CanonicalRootRole, } - req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))} + for _, keyHandler := range []simplerHandler{getKeyHandler, rotateKeyHandler} { + var buf bytes.Buffer - err := getKeyHandler(getContext(state), httptest.NewRecorder(), req, vars) - assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid role") + err := keyHandler(getContext(state), &buf, vars) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid role") + } } // Getting the key for a valid role and gun succeeds func TestGetKeyHandlerCreatesOnce(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 := getKeyHandler(getContext(state), recorder, req, vars) + var buf bytes.Buffer + err := getKeyHandler(getContext(state), &buf, vars) assert.NoError(t, err) - assert.True(t, len(recorder.Body.String()) > 0) + assert.NotEmpty(t, buf.Bytes()) } } diff --git a/server/server.go b/server/server.go index 98d1d9c68..6a4bc9fff 100644 --- a/server/server.go +++ b/server/server.go @@ -102,6 +102,11 @@ func RootHandler(ac auth.AccessController, ctx context.Context, trust signed.Cry prometheus.InstrumentHandlerWithOpts( prometheusOpts("GetKey"), hand(handlers.GetKeyHandler, "push", "pull"))) + r.Methods("POST").Path( + "/v2/{imageName:.*}/_trust/tuf/{tufRole:snapshot|timestamp}.key").Handler( + prometheus.InstrumentHandlerWithOpts( + prometheusOpts("RotateKey"), + hand(handlers.RotateKeyHandler, "push", "pull"))) r.Methods("DELETE").Path("/v2/{imageName:.*}/_trust/tuf/").Handler( prometheus.InstrumentHandlerWithOpts( prometheusOpts("DeleteTuf"), diff --git a/server/server_test.go b/server/server_test.go index 81d9724f6..4971425ef 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1,6 +1,7 @@ package server import ( + "bytes" "crypto/sha256" "encoding/hex" "encoding/json" @@ -90,6 +91,34 @@ func TestGetKeysEndpoint(t *testing.T) { } } +// RotateKey supports only the timestamp and snapshot key endpoints +func TestRotateKeyEndpoint(t *testing.T) { + ctx := context.WithValue( + context.Background(), "metaStore", storage.NewMemStorage()) + ctx = context.WithValue(ctx, "keyAlgorithm", data.ED25519Key) + + handler := RootHandler(nil, ctx, signed.NewEd25519()) + 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, + "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) + assert.NoError(t, err) + assert.Equal(t, expectedStatus, res.StatusCode) + } +} + // This just checks the URL routing is working correctly. // More detailed tests for this path including negative // tests are located in /server/handlers/ diff --git a/tuf/store/httpstore.go b/tuf/store/httpstore.go index 8b0d85011..532ae92d3 100644 --- a/tuf/store/httpstore.go +++ b/tuf/store/httpstore.go @@ -21,9 +21,12 @@ import ( "net/http" "net/url" "path" + "time" "github.com/Sirupsen/logrus" "github.com/docker/notary" + "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/validation" ) @@ -131,6 +134,8 @@ func translateStatusToError(resp *http.Response, resource string) error { return ErrMetaNotFound{Resource: resource} case http.StatusBadRequest: return tryUnmarshalError(resp, ErrInvalidOperation{}) + case notary.HTTPStatusTooManyRequests: + return ErrInvalidOperation{fmt.Sprintf("%s rate limited", resource)} default: return ErrServerUnavailable{code: resp.StatusCode} } @@ -270,13 +275,47 @@ func (s HTTPStore) buildURL(uri string) (*url.URL, error) { return s.baseURL.ResolveReference(sub), nil } -// GetKey retrieves a public key from the remote server -func (s HTTPStore) GetKey(role string) ([]byte, error) { +// GetKey retrieves the most recently created (whether it is signed in yet or not) +// public key for the given role from the remote server +func (s HTTPStore) GetKey(role string) (data.PublicKey, error) { + return s.requestKey(role, "GET", fmt.Sprintf("%s key", role), nil) +} + +// RotateKey rotates a key on the remote server and returns the new public key. This requires +// a request with a short expiry time (to limit replay), signed by at least one root key. +// Signing with the root key proves that the client making the request has the capability of +// actually rotating the key, and is not just making a spurious rotation request. +func (s HTTPStore) RotateKey(role string, cs signed.CryptoService, roots ...data.PublicKey) (data.PublicKey, error) { + req := &data.SignedCommon{ + Type: role, + Expires: time.Now().Add(5 * time.Minute), + } + reqJSON, err := json.Marshal(req) + if err != nil { + return nil, err + } + signedReq := &data.Signed{Signed: reqJSON} + if err := signed.Sign(cs, signedReq, roots...); err != nil { + return nil, err + } + + requestBody, err := json.Marshal(signedReq) + if err != nil { + return nil, err + } + + return s.requestKey(role, "POST", fmt.Sprintf("%s key rotation", role), bytes.NewBuffer(requestBody)) +} + +// requestKey either sends a get or a post request, depending on whether there +// is a body. +func (s HTTPStore) requestKey(role, method, resource string, body io.Reader) (data.PublicKey, error) { url, err := s.buildKeyURL(role) if err != nil { return nil, err } - req, err := http.NewRequest("GET", url.String(), nil) + + req, err := http.NewRequest(method, url.String(), body) if err != nil { return nil, err } @@ -285,12 +324,18 @@ func (s HTTPStore) GetKey(role string) ([]byte, error) { return nil, err } defer resp.Body.Close() - if err := translateStatusToError(resp, role+" key"); err != nil { + if err := translateStatusToError(resp, resource); err != nil { return nil, err } - body, err := ioutil.ReadAll(resp.Body) + respBody, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, err } - return body, nil + + pubKey, err := data.UnmarshalPublicKey(respBody) + if err != nil { + return nil, err + } + + return pubKey, nil } diff --git a/tuf/store/httpstore_test.go b/tuf/store/httpstore_test.go index 7ce3fefb5..363c8cfe5 100644 --- a/tuf/store/httpstore_test.go +++ b/tuf/store/httpstore_test.go @@ -7,17 +7,20 @@ import ( "fmt" "io" "io/ioutil" + "net" "net/http" "net/http/httptest" "strings" "testing" - - "github.com/stretchr/testify/assert" + "time" "github.com/docker/go/canonical/json" + "github.com/docker/notary" "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/keys" "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/validation" + "github.com/stretchr/testify/assert" ) const testRoot = `{"signed":{"_type":"Root","consistent_snapshot":false,"expires":"2025-07-17T16:19:21.101698314-07:00","keys":{"1ca15c7f4b2b0c6efce202a545e7267152da28ab7c91590b3b60bdb4da723aad":{"keytype":"ecdsa","keyval":{"private":null,"public":"MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEb0720c99Cj6ZmuDlznEZ52NA6YpeY9Sj45z51XvPnG63Bi2RSBezMJlPzbSfP39mXKXqOJyT+z9BZhi3FVWczg=="}},"b1d6813b55442ecbfb1f4b40eb1fcdb4290e53434cfc9ba2da24c26c9143873b":{"keytype":"ecdsa-x509","keyval":{"private":null,"public":"LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJVekNCKzZBREFnRUNBaEFCWDNKLzkzaW8zbHcrZUsvNFhvSHhNQW9HQ0NxR1NNNDlCQU1DTUJFeER6QU4KQmdOVkJBTVRCbVY0Y0dseVpUQWVGdzB4TlRBM01qQXlNekU1TVRkYUZ3MHlOVEEzTVRjeU16RTVNVGRhTUJFeApEekFOQmdOVkJBTVRCbVY0Y0dseVpUQlpNQk1HQnlxR1NNNDlBZ0VHQ0NxR1NNNDlBd0VIQTBJQUJFTDhOTFhQCitreUJZYzhYY0FTMXB2S2l5MXRQUDlCZHJ1dEdrWlR3Z0dEYTM1THMzSUFXaWlrUmlPbGRuWmxVVEE5cG5JekoKOFlRQThhTjQ1TDQvUlplak5UQXpNQTRHQTFVZER3RUIvd1FFQXdJQW9EQVRCZ05WSFNVRUREQUtCZ2dyQmdFRgpCUWNEQXpBTUJnTlZIUk1CQWY4RUFqQUFNQW9HQ0NxR1NNNDlCQU1DQTBjQU1FUUNJRVJ1ZUVURG5xMlRqRFBmClhGRStqUFJqMEtqdXdEOG9HSmtoVGpMUDAycjhBaUI5cUNyL2ZqSXpJZ1NQcTJVSXZqR0hlYmZOYXh1QlpZZUUKYW8xNjd6dHNYZz09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"}},"fbddae7f25a6c23ca735b017206a849d4c89304a4d8de4dcc4b3d6f3eb22ce3b":{"keytype":"ecdsa","keyval":{"private":null,"public":"MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE/xS5fBHK2HKmlGcvAr06vwPITvmxWP4P3CMDCgY25iSaIiM21OiXA1/Uvo3Pa3xh5G3cwCtDvi+4FpflW2iB/w=="}},"fd75751f010c3442e23b3e3e99a1442a112f2f21038603cb8609d8b17c9e912a":{"keytype":"ed25519","keyval":{"private":null,"public":"rc+glN01m+q8jmX8SolGsjTfk6NMhUQTWyj10hjmne0="}}},"roles":{"root":{"keyids":["b1d6813b55442ecbfb1f4b40eb1fcdb4290e53434cfc9ba2da24c26c9143873b"],"threshold":1},"snapshot":{"keyids":["1ca15c7f4b2b0c6efce202a545e7267152da28ab7c91590b3b60bdb4da723aad"],"threshold":1},"targets":{"keyids":["fbddae7f25a6c23ca735b017206a849d4c89304a4d8de4dcc4b3d6f3eb22ce3b"],"threshold":1},"timestamp":{"keyids":["fd75751f010c3442e23b3e3e99a1442a112f2f21038603cb8609d8b17c9e912a"],"threshold":1}},"version":2},"signatures":[{"keyid":"b1d6813b55442ecbfb1f4b40eb1fcdb4290e53434cfc9ba2da24c26c9143873b","method":"ecdsa","sig":"A2lNVwxHBnD9ViFtRre8r5oG6VvcvJnC6gdvvxv/Jyag40q/fNMjllCqyHrb+6z8XDZcrTTDsFU1R3/e+92d1A=="}]}` @@ -324,3 +327,173 @@ func TestErrServerUnavailable(t *testing.T) { } } } + +// GetKey and RotateKey both succeed if valid public keys are returned from the server with a 200 status +func TestGetKeyAndRotateKeySuccess(t *testing.T) { + c := signed.NewEd25519() + role := data.CanonicalSnapshotRole + key, err := c.Create(role, data.ED25519Key) + assert.NoError(t, err) + + keyJSON, err := json.Marshal(key) + assert.NoError(t, err) + + // Set up a simple handler and server for our store + handler := func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(keyJSON)) + } + server := httptest.NewServer(http.HandlerFunc(handler)) + defer server.Close() + store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport) + assert.NoError(t, err) + + gotten, err := store.GetKey(role) + assert.NoError(t, err) + assert.Equal(t, key.ID(), gotten.ID()) + + rotated, err := store.RotateKey(role, c, key) + assert.NoError(t, err) + assert.Equal(t, key.ID(), rotated.ID()) +} + +// GetKey and RotateKey both fail if the key returned is invalid JSON +func TestGetKeyAndRotateKeyInvalidJSON(t *testing.T) { + role := data.CanonicalSnapshotRole + + c := signed.NewEd25519() + root, err := c.Create(data.CanonicalRootRole, data.ED25519Key) + assert.NoError(t, err) + + // Set up a simple handler and server for our store + handler := func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("{")) + } + server := httptest.NewServer(http.HandlerFunc(handler)) + defer server.Close() + store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport) + assert.NoError(t, err) + + _, err = store.GetKey(role) + assert.Error(t, err) + assert.IsType(t, &json.SyntaxError{}, err) + + _, err = store.RotateKey(role, c, root) + assert.Error(t, err) + assert.IsType(t, &json.SyntaxError{}, err) +} + +// GetKey and RotateKey both fail if it cannot make the request +func TestGetKeyAndRotateKeyServerUnreachable(t *testing.T) { + role := data.CanonicalSnapshotRole + + c := signed.NewEd25519() + root, err := c.Create(data.CanonicalRootRole, data.ED25519Key) + assert.NoError(t, err) + + // nonexistent URL + store, err := NewHTTPStore("http://localhost:90861", "metadata", "json", "key", http.DefaultTransport) + assert.NoError(t, err) + + _, err = store.GetKey(role) + assert.Error(t, err) + assert.IsType(t, &net.OpError{}, err) + + _, err = store.RotateKey(role, c, root) + assert.Error(t, err) + assert.IsType(t, &net.OpError{}, err) +} + +// GetKey and RotateKey both fail with ErrInvalidOperation if a notary.HTTPStatusTooManyRequests is returned +func TestGetKeyAndRotateKeyServerLimitError(t *testing.T) { + role := data.CanonicalSnapshotRole + + c := signed.NewEd25519() + root, err := c.Create(data.CanonicalRootRole, data.ED25519Key) + assert.NoError(t, err) + + // Set up a simple handler and server for our store + handler := func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(notary.HTTPStatusTooManyRequests) + } + server := httptest.NewServer(http.HandlerFunc(handler)) + defer server.Close() + store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport) + assert.NoError(t, err) + + _, err = store.GetKey(role) + assert.Error(t, err) + assert.IsType(t, ErrInvalidOperation{}, err) + + _, err = store.RotateKey(role, c, root) + assert.Error(t, err) + assert.IsType(t, ErrInvalidOperation{}, err) +} + +// RotateKey sends a signed request to the the server to rotate a key +func TestRotateKeySendsSignedRequest(t *testing.T) { + role := data.CanonicalSnapshotRole + + c := signed.NewEd25519() + root, err := c.Create(data.CanonicalRootRole, data.ED25519Key) + assert.NoError(t, err) + snapshot, err := c.Create(role, data.ED25519Key) + assert.NoError(t, err) + + keyJSON, err := json.Marshal(snapshot) + assert.NoError(t, err) + + kdb := keys.NewDB() + kdb.AddKey(root) + roleObj, err := data.NewRole(data.CanonicalRootRole, 1, []string{root.ID()}, nil, nil) + assert.NoError(t, err) + kdb.AddRole(roleObj) + + // Set up handler that asserts information about the request + m := http.NewServeMux() + m.HandleFunc("/metadata/snapshot.key", func(w http.ResponseWriter, r *http.Request) { + decoder := json.NewDecoder(r.Body) + s := data.Signed{} + assert.NoError(t, decoder.Decode(&s)) + assert.NoError(t, signed.VerifySignatures(&s, data.CanonicalRootRole, kdb)) + common := data.SignedCommon{} + assert.NoError(t, json.Unmarshal(s.Signed, &common)) + assert.Equal(t, data.CanonicalSnapshotRole, common.Type) + assert.True(t, common.Expires.After(time.Now())) + assert.False(t, common.Expires.After(time.Now().Add(5*time.Minute))) + + w.Write(keyJSON) + }) + server := httptest.NewServer(m) + defer server.Close() + + store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport) + assert.NoError(t, err) + + k, err := store.RotateKey(role, c, root) + assert.NoError(t, err) + assert.Equal(t, snapshot.ID(), k.ID()) +} + +// RotateKey fails to even send a request if we can't sign. +func TestRotateKeyDoesntSendRequestIfCannotSign(t *testing.T) { + role := data.CanonicalSnapshotRole + + c := signed.NewEd25519() + root, err := c.Create(data.CanonicalRootRole, data.ED25519Key) + assert.NoError(t, err) + + // Set up handler that asserts information about the request + m := http.NewServeMux() + m.HandleFunc("/metadata/snapshot.key", func(w http.ResponseWriter, r *http.Request) { + assert.Fail(t, "No request should ever have been made.") + }) + server := httptest.NewServer(m) + defer server.Close() + + store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport) + assert.NoError(t, err) + + _, err = store.RotateKey(role, signed.NewEd25519(), root) + assert.Error(t, err) + assert.IsType(t, signed.ErrNoKeys{}, err) +} diff --git a/tuf/store/interfaces.go b/tuf/store/interfaces.go index dd307168b..96bb3c3e4 100644 --- a/tuf/store/interfaces.go +++ b/tuf/store/interfaces.go @@ -1,5 +1,10 @@ package store +import ( + "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/signed" +) + // MetadataStore must be implemented by anything that intends to interact // with a store of TUF files type MetadataStore interface { @@ -12,7 +17,8 @@ type MetadataStore interface { // PublicKeyStore must be implemented by a key service type PublicKeyStore interface { - GetKey(role string) ([]byte, error) + GetKey(role string) (data.PublicKey, error) + RotateKey(role string, cs signed.CryptoService, roots ...data.PublicKey) (data.PublicKey, error) } // LocalStore represents a local TUF sture diff --git a/tuf/store/memorystore.go b/tuf/store/memorystore.go index 493bb6f0b..be91e7af3 100644 --- a/tuf/store/memorystore.go +++ b/tuf/store/memorystore.go @@ -6,6 +6,7 @@ import ( "github.com/docker/notary" "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/utils" ) @@ -94,9 +95,14 @@ func (m *MemoryStore) RemoveMeta(name string) error { return nil } -// GetKey returns the public key for the given role -func (m *MemoryStore) GetKey(role string) ([]byte, error) { - return nil, fmt.Errorf("GetKey is not implemented for the MemoryStore") +// GetKey returns the latest key for a given role +func (m *MemoryStore) GetKey(role string) (data.PublicKey, error) { + return nil, fmt.Errorf("GetKey is not implemented for the memoryStore") +} + +// RotateKey rotates a key for a given role +func (m *MemoryStore) RotateKey(string, signed.CryptoService, ...data.PublicKey) (data.PublicKey, error) { + return nil, fmt.Errorf("RotateKey is not implemented for the memoryStore") } // RemoveAll clears the existing memory store by setting this store as new empty one diff --git a/tuf/store/memorystore_test.go b/tuf/store/memorystore_test.go index 64eee5163..8d4dabfee 100644 --- a/tuf/store/memorystore_test.go +++ b/tuf/store/memorystore_test.go @@ -3,6 +3,7 @@ package store import ( "testing" + "github.com/docker/notary/tuf/data" "github.com/stretchr/testify/require" ) @@ -31,4 +32,10 @@ func TestMemoryStore(t *testing.T) { _, err = s.GetMeta("exists", 0) require.Error(t, err) require.IsType(t, ErrMetaNotFound{}, err) + + _, err = s.GetKey(data.CanonicalSnapshotRole) + require.Error(t, err) + + _, err = s.RotateKey(data.CanonicalSnapshotRole, nil) + require.Error(t, err) } diff --git a/tuf/store/offlinestore.go b/tuf/store/offlinestore.go index b0f057b2b..624ff52be 100644 --- a/tuf/store/offlinestore.go +++ b/tuf/store/offlinestore.go @@ -2,6 +2,9 @@ package store import ( "io" + + "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/signed" ) // ErrOffline is used to indicate we are operating offline @@ -11,7 +14,7 @@ func (e ErrOffline) Error() string { return "client is offline" } -var err = ErrOffline{} +var errOffline = ErrOffline{} // OfflineStore is to be used as a placeholder for a nil store. It simply // returns ErrOffline for every operation @@ -19,35 +22,40 @@ type OfflineStore struct{} // GetMeta returns ErrOffline func (es OfflineStore) GetMeta(name string, size int64) ([]byte, error) { - return nil, err + return nil, errOffline } // SetMeta returns ErrOffline func (es OfflineStore) SetMeta(name string, blob []byte) error { - return err + return errOffline } // SetMultiMeta returns ErrOffline func (es OfflineStore) SetMultiMeta(map[string][]byte) error { - return err + return errOffline } // RemoveMeta returns ErrOffline func (es OfflineStore) RemoveMeta(name string) error { - return err + return errOffline } // GetKey returns ErrOffline -func (es OfflineStore) GetKey(role string) ([]byte, error) { - return nil, err +func (es OfflineStore) GetKey(role string) (data.PublicKey, error) { + return nil, errOffline +} + +// RotateKey returns ErrOffline +func (es OfflineStore) RotateKey(string, signed.CryptoService, ...data.PublicKey) (data.PublicKey, error) { + return nil, errOffline } // GetTarget returns ErrOffline func (es OfflineStore) GetTarget(path string) (io.ReadCloser, error) { - return nil, err + return nil, errOffline } // RemoveAll return ErrOffline func (es OfflineStore) RemoveAll() error { - return err + return errOffline } diff --git a/tuf/store/offlinestore_test.go b/tuf/store/offlinestore_test.go index 66de915e7..e832f6c33 100644 --- a/tuf/store/offlinestore_test.go +++ b/tuf/store/offlinestore_test.go @@ -24,6 +24,10 @@ func TestOfflineStore(t *testing.T) { require.Error(t, err) require.IsType(t, ErrOffline{}, err) + _, err = s.RotateKey("", nil) + require.Error(t, err) + require.IsType(t, ErrOffline{}, err) + _, err = s.GetTarget("") require.Error(t, err) require.IsType(t, ErrOffline{}, err) diff --git a/tuf/tuf.go b/tuf/tuf.go index 8001c45de..0da0963e9 100644 --- a/tuf/tuf.go +++ b/tuf/tuf.go @@ -649,12 +649,11 @@ func (tr *Repo) SignRoot(expires time.Time) (*data.Signed, error) { logrus.Debug("signing root...") tr.Root.Signed.Expires = expires tr.Root.Signed.Version++ - root := tr.keysDB.GetRole(data.CanonicalRootRole) signed, err := tr.Root.ToSigned() if err != nil { return nil, err } - signed, err = tr.sign(signed, *root) + signed, err = tr.sign(signed, data.CanonicalRootRole) if err != nil { return nil, err } @@ -678,8 +677,7 @@ func (tr *Repo) SignTargets(role string, expires time.Time) (*data.Signed, error logrus.Debug("errored getting targets data.Signed object") return nil, err } - targets := tr.keysDB.GetRole(role) - signed, err = tr.sign(signed, *targets) + signed, err = tr.sign(signed, role) if err != nil { logrus.Debug("errored signing ", role) return nil, err @@ -717,8 +715,7 @@ func (tr *Repo) SignSnapshot(expires time.Time) (*data.Signed, error) { if err != nil { return nil, err } - snapshot := tr.keysDB.GetRole(data.CanonicalSnapshotRole) - signed, err = tr.sign(signed, *snapshot) + signed, err = tr.sign(signed, data.CanonicalSnapshotRole) if err != nil { return nil, err } @@ -743,8 +740,7 @@ func (tr *Repo) SignTimestamp(expires time.Time) (*data.Signed, error) { if err != nil { return nil, err } - timestamp := tr.keysDB.GetRole(data.CanonicalTimestampRole) - signed, err = tr.sign(signed, *timestamp) + signed, err = tr.sign(signed, data.CanonicalTimestampRole) if err != nil { return nil, err } @@ -753,9 +749,14 @@ func (tr *Repo) SignTimestamp(expires time.Time) (*data.Signed, error) { return signed, nil } -func (tr Repo) sign(signedData *data.Signed, role data.Role) (*data.Signed, error) { - ks := make([]data.PublicKey, 0, len(role.KeyIDs)) - for _, kid := range role.KeyIDs { +// SigningKeysForRole returns the public keys necessary for signing a particular role +func (tr *Repo) SigningKeysForRole(role string) ([]data.PublicKey, error) { + roleObj := tr.keysDB.GetRole(role) + if roleObj == nil { + return nil, data.ErrInvalidRole{Role: role, Reason: "does not exist"} + } + ks := make([]data.PublicKey, 0, len(roleObj.KeyIDs)) + for _, kid := range roleObj.KeyIDs { k := tr.keysDB.GetKey(kid) if k == nil { continue @@ -765,9 +766,16 @@ func (tr Repo) sign(signedData *data.Signed, role data.Role) (*data.Signed, erro if len(ks) < 1 { return nil, keys.ErrInvalidKey } - err := signed.Sign(tr.cryptoService, signedData, ks...) + return ks, nil +} + +func (tr Repo) sign(signedData *data.Signed, role string) (*data.Signed, error) { + signingKeys, err := tr.SigningKeysForRole(role) if err != nil { return nil, err } + if err := signed.Sign(tr.cryptoService, signedData, signingKeys...); err != nil { + return nil, err + } return signedData, nil }