From c0466148b552acb3b09ecb535e3f2df973c705a1 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Tue, 14 Feb 2017 08:59:30 -0800 Subject: [PATCH 01/19] Add lazy initialization of a repository at publish time Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 19 ++++++---- cmd/notary/tuf.go | 94 +++++++++++++++++++++++++++++------------------ 2 files changed, 70 insertions(+), 43 deletions(-) diff --git a/client/client.go b/client/client.go index 2a27e32a0..728fda6d3 100644 --- a/client/client.go +++ b/client/client.go @@ -120,6 +120,11 @@ func NewNotaryRepository(baseDir string, gun data.GUN, baseURL string, remoteSto return nRepo, nil } +// GetGUN is a getter for the GUN object from a NotaryRepository +func (r *NotaryRepository) GetGUN() string { + return r.gun +} + // Target represents a simplified version of the data TUF operates on, so external // applications don't have to depend on TUF data types. type Target struct { @@ -613,22 +618,20 @@ func (r *NotaryRepository) Publish() error { // publish pushes the changes in the given changelist to the remote notary-server // Conceptually it performs an operation similar to a `git rebase` -func (r *NotaryRepository) publish(cl changelist.Changelist) error { +func (r *NotaryRepository) publish(cl changelist.Changelist, rootKeyIDs []string) error { var initialPublish bool // update first before publishing if err := r.Update(true); err != nil { // If the remote is not aware of the repo, then this is being published - // for the first time. Try to load from disk instead for publishing. + // for the first time. Try to initialize the repository before publishing. if _, ok := err.(ErrRepositoryNotExist); ok { - err := r.bootstrapRepo() + err := r.Initialize(rootKeyIDs) if err != nil { - logrus.Debugf("Unable to load repository from local files: %s", + logrus.Debugf("Unable to initialize repository at publish-time: %s", err.Error()) - if _, ok := err.(store.ErrMetaNotFound); ok { - return ErrRepoNotInitialized{} - } return err } + // Ensure we will push the initial root and targets file. Either or // both of the root and targets may not be marked as Dirty, since // there may not be any changes that update them, so use a @@ -1029,7 +1032,7 @@ func (r *NotaryRepository) RotateKey(role data.RoleName, serverManagesKey bool, if err := r.rootFileKeyChange(cl, role, changelist.ActionCreate, pubKeyList); err != nil { return err } - return r.publish(cl) + return r.publish(cl, []string{r.GetGUN()}) } // Given a set of new keys to rotate to and a set of keys to drop, returns the list of current keys to use diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index c80509a71..d9f715ebe 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -142,7 +142,10 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdReset.Flags().BoolVar(&t.resetAll, "all", false, "Reset all changes shown in the status list") cmd.AddCommand(cmdReset) - cmd.AddCommand(cmdTUFPublishTemplate.ToCommand(t.tufPublish)) + cmdTUFPublish := cmdTUFPublishTemplate.ToCommand(t.tufPublish) + cmdTUFPublish.Flags().StringVar(&t.rootKey, "rootkey", "", "Root key to initialize the repository with") + cmd.AddCommand(cmdTUFPublish) + cmd.AddCommand(cmdTUFLookupTemplate.ToCommand(t.tufLookup)) cmdTUFList := cmdTUFListTemplate.ToCommand(t.tufList) @@ -385,6 +388,42 @@ func (t *tufCommander) tufDeleteGUN(cmd *cobra.Command, args []string) error { return nil } +func importRootKey(cmd *cobra.Command,rootKey string, nRepo *notaryclient.NotaryRepository, retriever notary.PassRetriever) ([]string, error) { + var rootKeyList []string + var err error + + if rootKey != "" { + privKey, err := readKey(data.CanonicalRootRole, rootKey, retriever) + if err != nil { + return nil, err + } + err = nRepo.CryptoService.AddKey(data.CanonicalRootRole, "", privKey) + if err != nil { + return nil, fmt.Errorf("Error importing key: %v", err) + } + rootKeyList = []string{privKey.ID()} + } else { + rootKeyList = nRepo.CryptoService.ListKeys(data.CanonicalRootRole) + } + + var rootKeyID string + if len(rootKeyList) < 1 { + cmd.Println("No root keys found. Generating a new root key...") + rootPublicKey, err := nRepo.CryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey) + if err != nil { + return nil, err + } + rootKeyID = rootPublicKey.ID() + } else { + // Chooses the first root key available, which is initialization specific + // but should return the HW one first. + rootKeyID = rootKeyList[0] + cmd.Printf("Root key found, using: %s\n", rootKeyID) + } + + return []string{rootKeyID}, err +} + func (t *tufCommander) tufInit(cmd *cobra.Command, args []string) error { if len(args) < 1 { cmd.Usage() @@ -413,38 +452,12 @@ func (t *tufCommander) tufInit(cmd *cobra.Command, args []string) error { return err } - var rootKeyList []string - - if t.rootKey != "" { - privKey, err := readKey(data.CanonicalRootRole, t.rootKey, t.retriever) - if err != nil { - return err - } - err = nRepo.CryptoService.AddKey(data.CanonicalRootRole, "", privKey) - if err != nil { - return fmt.Errorf("Error importing key: %v", err) - } - rootKeyList = []string{privKey.ID()} - } else { - rootKeyList = nRepo.CryptoService.ListKeys(data.CanonicalRootRole) - } - - var rootKeyID string - if len(rootKeyList) < 1 { - cmd.Println("No root keys found. Generating a new root key...") - rootPublicKey, err := nRepo.CryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey) - if err != nil { - return err - } - rootKeyID = rootPublicKey.ID() - } else { - // Chooses the first root key available, which is initialization specific - // but should return the HW one first. - rootKeyID = rootKeyList[0] - cmd.Printf("Root key found, using: %s\n", rootKeyID) + rootKeyIDs, err := importRootKey(cmd, t.rootKey, nRepo, t.retriever) + if err != nil { + return err } - if err = nRepo.Initialize([]string{rootKeyID}); err != nil { + if err = nRepo.Initialize(rootKeyIDs); err != nil { return err } @@ -686,7 +699,18 @@ func (t *tufCommander) tufPublish(cmd *cobra.Command, args []string) error { return err } - return publishAndPrintToCLI(cmd, nRepo, gun) + var rootKeyIDs []string + + if err := nRepo.Update(true); err != nil { + if _, ok := err.(notaryclient.ErrRepositoryNotExist); ok { + rootKeyIDs, err = importRootKey(cmd, t.rootKey, nRepo, t.retriever) + if err != nil { + return err + } + } + } + + return publishAndPrintToCLI(cmd, nRepo, rootKeyIDs) } func (t *tufCommander) tufRemove(cmd *cobra.Command, args []string) error { @@ -1031,14 +1055,14 @@ func maybeAutoPublish(cmd *cobra.Command, doPublish bool, gun data.GUN, config * return err } - cmd.Println("Auto-publishing changes to", gun) - return publishAndPrintToCLI(cmd, nRepo, gun) + cmd.Println("Auto-publishing changes to", nRepo.GetGUN()) + return publishAndPrintToCLI(cmd, nRepo, nil) } func publishAndPrintToCLI(cmd *cobra.Command, nRepo *notaryclient.NotaryRepository, gun data.GUN) error { if err := nRepo.Publish(); err != nil { return err } - cmd.Printf("Successfully published changes for repository %s\n", gun) + cmd.Printf("Successfully published changes for repository %s\n", nRepo.GetGUN()) return nil } From 9a4d4b8ce19d4b727b84e37e261074900eda85b3 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Tue, 14 Feb 2017 10:50:18 -0800 Subject: [PATCH 02/19] Syntax fix by gofmt Signed-off-by: Nassim 'Nass' Eddequiouaq --- cmd/notary/tuf.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index d9f715ebe..a4fadc859 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -388,7 +388,7 @@ func (t *tufCommander) tufDeleteGUN(cmd *cobra.Command, args []string) error { return nil } -func importRootKey(cmd *cobra.Command,rootKey string, nRepo *notaryclient.NotaryRepository, retriever notary.PassRetriever) ([]string, error) { +func importRootKey(cmd *cobra.Command, rootKey string, nRepo *notaryclient.NotaryRepository, retriever notary.PassRetriever) ([]string, error) { var rootKeyList []string var err error From d11ad89729b7b3273a6b1a5b47e8abb6a2b5e2fb Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Tue, 14 Feb 2017 08:59:30 -0800 Subject: [PATCH 03/19] Add lazy initialization of a repository at publish time Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/client.go b/client/client.go index 728fda6d3..88076c86d 100644 --- a/client/client.go +++ b/client/client.go @@ -603,8 +603,8 @@ func (r *NotaryRepository) ListRoles() ([]RoleWithSignatures, error) { // Publish pushes the local changes in signed material to the remote notary-server // Conceptually it performs an operation similar to a `git rebase` -func (r *NotaryRepository) Publish() error { - if err := r.publish(r.changelist); err != nil { +func (r *NotaryRepository) Publish(rootKeyIDs []string) error { + if err := r.publish(r.changelist, rootKeyIDs); err != nil { return err } if err := r.changelist.Clear(""); err != nil { From dd059a67c7d1edd41e0ad74577ad404197bfcddd Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 20 Feb 2017 15:46:30 -0800 Subject: [PATCH 04/19] lots of failing tests Signed-off-by: David Lawrence (github: endophage) --- client/client.go | 29 +++++++++++++++++++++++------ client/client_test.go | 1 + cmd/notary/tuf.go | 24 +++--------------------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/client/client.go b/client/client.go index 88076c86d..ca1088554 100644 --- a/client/client.go +++ b/client/client.go @@ -187,6 +187,24 @@ func (r *NotaryRepository) Initialize(rootKeyIDs []string, serverManagedRoles .. } privKeys = append(privKeys, privKey) } + if len(privKeys) == 0 { + var rootKeyID string + rootKeyList := r.CryptoService.ListKeys(data.CanonicalRootRole) + if len(rootKeyList) == 0 { + rootPublicKey, err := r.CryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey) + if err != nil { + return err + } + rootKeyID = rootPublicKey.ID() + } else { + rootKeyID = rootKeyList[0] + } + privKey, _, err := r.CryptoService.GetPrivateKey(rootKeyID) + if err != nil { + return err + } + privKeys = append(privKeys, privKey) + } // currently we only support server managing timestamps and snapshots, and // nothing else - timestamps are always managed by the server, and implicit @@ -259,7 +277,6 @@ func (r *NotaryRepository) Initialize(rootKeyIDs []string, serverManagedRoles .. func (r *NotaryRepository) initializeRoles(rootKeys []data.PublicKey, localRoles, remoteRoles []data.RoleName) ( root, targets, snapshot, timestamp data.BaseRole, err error) { - root = data.NewBaseRole( data.CanonicalRootRole, notary.MinThreshold, @@ -603,8 +620,8 @@ func (r *NotaryRepository) ListRoles() ([]RoleWithSignatures, error) { // Publish pushes the local changes in signed material to the remote notary-server // Conceptually it performs an operation similar to a `git rebase` -func (r *NotaryRepository) Publish(rootKeyIDs []string) error { - if err := r.publish(r.changelist, rootKeyIDs); err != nil { +func (r *NotaryRepository) Publish() error { + if err := r.publish(r.changelist); err != nil { return err } if err := r.changelist.Clear(""); err != nil { @@ -618,14 +635,14 @@ func (r *NotaryRepository) Publish(rootKeyIDs []string) error { // publish pushes the changes in the given changelist to the remote notary-server // Conceptually it performs an operation similar to a `git rebase` -func (r *NotaryRepository) publish(cl changelist.Changelist, rootKeyIDs []string) error { +func (r *NotaryRepository) publish(cl changelist.Changelist) error { var initialPublish bool // update first before publishing if err := r.Update(true); err != nil { // If the remote is not aware of the repo, then this is being published // for the first time. Try to initialize the repository before publishing. if _, ok := err.(ErrRepositoryNotExist); ok { - err := r.Initialize(rootKeyIDs) + err := r.Initialize(nil) if err != nil { logrus.Debugf("Unable to initialize repository at publish-time: %s", err.Error()) @@ -1032,7 +1049,7 @@ func (r *NotaryRepository) RotateKey(role data.RoleName, serverManagesKey bool, if err := r.rootFileKeyChange(cl, role, changelist.ActionCreate, pubKeyList); err != nil { return err } - return r.publish(cl, []string{r.GetGUN()}) + return r.publish(cl) } // Given a set of new keys to rotate to and a set of keys to drop, returns the list of current keys to use diff --git a/client/client_test.go b/client/client_test.go index 557639070..d6502248b 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1684,6 +1684,7 @@ func TestPublishUninitializedRepo(t *testing.T) { requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true) requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true) + // check we can just call publish again require.NoError(t, repo.Publish()) } diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index a4fadc859..6d51ce9c2 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -407,14 +407,7 @@ func importRootKey(cmd *cobra.Command, rootKey string, nRepo *notaryclient.Notar } var rootKeyID string - if len(rootKeyList) < 1 { - cmd.Println("No root keys found. Generating a new root key...") - rootPublicKey, err := nRepo.CryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey) - if err != nil { - return nil, err - } - rootKeyID = rootPublicKey.ID() - } else { + if len(rootKeyList) > 0 { // Chooses the first root key available, which is initialization specific // but should return the HW one first. rootKeyID = rootKeyList[0] @@ -699,18 +692,7 @@ func (t *tufCommander) tufPublish(cmd *cobra.Command, args []string) error { return err } - var rootKeyIDs []string - - if err := nRepo.Update(true); err != nil { - if _, ok := err.(notaryclient.ErrRepositoryNotExist); ok { - rootKeyIDs, err = importRootKey(cmd, t.rootKey, nRepo, t.retriever) - if err != nil { - return err - } - } - } - - return publishAndPrintToCLI(cmd, nRepo, rootKeyIDs) + return publishAndPrintToCLI(cmd, nRepo) } func (t *tufCommander) tufRemove(cmd *cobra.Command, args []string) error { @@ -1056,7 +1038,7 @@ func maybeAutoPublish(cmd *cobra.Command, doPublish bool, gun data.GUN, config * } cmd.Println("Auto-publishing changes to", nRepo.GetGUN()) - return publishAndPrintToCLI(cmd, nRepo, nil) + return publishAndPrintToCLI(cmd, nRepo) } func publishAndPrintToCLI(cmd *cobra.Command, nRepo *notaryclient.NotaryRepository, gun data.GUN) error { From e556e9464bb72b71cee4d9689e23152b376d04ba Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 24 Feb 2017 13:42:15 +0100 Subject: [PATCH 05/19] Fix rootkey IDs list creation causing faulty access at repo init Signed-off-by: Nassim 'Nass' Eddequiouaq --- cmd/notary/tuf.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 6d51ce9c2..1272cae23 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -390,7 +390,6 @@ func (t *tufCommander) tufDeleteGUN(cmd *cobra.Command, args []string) error { func importRootKey(cmd *cobra.Command, rootKey string, nRepo *notaryclient.NotaryRepository, retriever notary.PassRetriever) ([]string, error) { var rootKeyList []string - var err error if rootKey != "" { privKey, err := readKey(data.CanonicalRootRole, rootKey, retriever) @@ -406,15 +405,16 @@ func importRootKey(cmd *cobra.Command, rootKey string, nRepo *notaryclient.Notar rootKeyList = nRepo.CryptoService.ListKeys(data.CanonicalRootRole) } - var rootKeyID string if len(rootKeyList) > 0 { // Chooses the first root key available, which is initialization specific // but should return the HW one first. - rootKeyID = rootKeyList[0] + rootKeyID := rootKeyList[0] cmd.Printf("Root key found, using: %s\n", rootKeyID) + + return []string{rootKeyID}, nil } - return []string{rootKeyID}, err + return []string{}, nil } func (t *tufCommander) tufInit(cmd *cobra.Command, args []string) error { From 5a9636a64c3791eed5f08ac36768f53dcd2a6936 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Wed, 22 Feb 2017 02:03:59 +0100 Subject: [PATCH 06/19] TestNotInitializedOnPublish doesn't work with repo lazy init at publish time Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client_test.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index d6502248b..58d2ba370 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1986,26 +1986,6 @@ func testPublishBadMetadata(t *testing.T, roleName string, repo *NotaryRepositor } } -// If the repo is not initialized, calling repo.Publish() should return ErrRepoNotInitialized -func TestNotInitializedOnPublish(t *testing.T) { - // Temporary directory where test files will be created - tempBaseDir, err := ioutil.TempDir("", "notary-test-") - defer os.RemoveAll(tempBaseDir) - require.NoError(t, err, "failed to create a temporary directory: %s", err) - - gun := "docker.com/notary" - ts := fullTestServer(t) - defer ts.Close() - - repo, _, _ := createRepoAndKey(t, data.ECDSAKey, tempBaseDir, gun, ts.URL) - - addTarget(t, repo, "v1", "../fixtures/intermediate-ca.crt") - - err = repo.Publish() - require.Error(t, err) - require.IsType(t, ErrRepoNotInitialized{}, err) -} - type cannotCreateKeys struct { signed.CryptoService } From 52a07d3f03409665499ee5dfb441302eed97e6eb Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Thu, 23 Feb 2017 02:39:24 +0100 Subject: [PATCH 07/19] Modify lazy init logic to bootstrap if metadata on disk Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/client/client.go b/client/client.go index ca1088554..07c6d3cf9 100644 --- a/client/client.go +++ b/client/client.go @@ -633,6 +633,23 @@ func (r *NotaryRepository) Publish() error { return nil } +func (r *NotaryRepository) isMetaCached() (bool, error) { + for _, role := range data.BaseRoles { + _, err := r.cache.GetSized(role, store.NoSizeLimit) + if err != nil { + if _, ok := err.(store.ErrMetaNotFound); ok { + continue + } + + return false, err + } + + return true, nil + } + + return false, nil +} + // publish pushes the changes in the given changelist to the remote notary-server // Conceptually it performs an operation similar to a `git rebase` func (r *NotaryRepository) publish(cl changelist.Changelist) error { @@ -642,7 +659,17 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { // If the remote is not aware of the repo, then this is being published // for the first time. Try to initialize the repository before publishing. if _, ok := err.(ErrRepositoryNotExist); ok { - err := r.Initialize(nil) + metaCached, err := r.isMetaCached() + if err != nil { + logrus.Debugf("Unable to verify on-disk cache: %s", err.Error()) + return err + } + + if metaCached { + err = r.bootstrapRepo() + } else { + err = r.Initialize(nil) + } if err != nil { logrus.Debugf("Unable to initialize repository at publish-time: %s", err.Error()) From 1da4d7ffcf903e06d18c53390fb540fdd408f75d Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Thu, 23 Feb 2017 02:57:34 +0100 Subject: [PATCH 08/19] Fix deprecated test accordingly to the new logic Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 58d2ba370..e7774ba92 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2631,8 +2631,10 @@ func TestRemoteRotationNoRootKey(t *testing.T) { require.IsType(t, signed.ErrInsufficientSignatures{}, err) } -// The repo hasn't been initialized, so we can't rotate -func TestRemoteRotationNonexistentRepo(t *testing.T) { +// The repo is initialized at publish time after +// rotating the key. We should be denied the access +// to metadata by the server when trying to retrieve it. +func TestRemoteRotationNoInit(t *testing.T) { ts, _, _ := simpleTestServer(t) defer ts.Close() @@ -2641,7 +2643,7 @@ func TestRemoteRotationNonexistentRepo(t *testing.T) { err := repo.RotateKey(data.CanonicalTimestampRole, true, nil) require.Error(t, err) - require.IsType(t, ErrRepoNotInitialized{}, err) + require.IsType(t, store.ErrMetaNotFound{}, err) } // Rotates the keys. After the rotation, downloading the latest metadata From 4182ca01366284a8a94472160a43b069ab746196 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 24 Feb 2017 14:31:45 +0100 Subject: [PATCH 09/19] Fix mishandled conflicts with types update PR Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 4 ++-- cmd/notary/tuf.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/client.go b/client/client.go index 07c6d3cf9..5102ca4d9 100644 --- a/client/client.go +++ b/client/client.go @@ -121,7 +121,7 @@ func NewNotaryRepository(baseDir string, gun data.GUN, baseURL string, remoteSto } // GetGUN is a getter for the GUN object from a NotaryRepository -func (r *NotaryRepository) GetGUN() string { +func (r *NotaryRepository) GetGUN() data.GUN { return r.gun } @@ -635,7 +635,7 @@ func (r *NotaryRepository) Publish() error { func (r *NotaryRepository) isMetaCached() (bool, error) { for _, role := range data.BaseRoles { - _, err := r.cache.GetSized(role, store.NoSizeLimit) + _, err := r.cache.GetSized(role.String(), store.NoSizeLimit) if err != nil { if _, ok := err.(store.ErrMetaNotFound); ok { continue diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 1272cae23..c9eda1a7a 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -1041,7 +1041,7 @@ func maybeAutoPublish(cmd *cobra.Command, doPublish bool, gun data.GUN, config * return publishAndPrintToCLI(cmd, nRepo) } -func publishAndPrintToCLI(cmd *cobra.Command, nRepo *notaryclient.NotaryRepository, gun data.GUN) error { +func publishAndPrintToCLI(cmd *cobra.Command, nRepo *notaryclient.NotaryRepository) error { if err := nRepo.Publish(); err != nil { return err } From afc4985b623dece1710a2f19a4f73534ab3000bd Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 24 Feb 2017 14:33:20 +0100 Subject: [PATCH 10/19] Update deprecated tests for repo lazy init behavior Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client_test.go | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index e7774ba92..7bcc3d2c6 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1650,14 +1650,13 @@ func testPublishNoData(t *testing.T, rootType string, clearCache, serverManagesS } } -// Publishing an uninitialized repo will fail, but initializing and republishing -// after should succeed +// Publishing an uninitialized repo should not fail func TestPublishUninitializedRepo(t *testing.T) { var gun data.GUN = "docker.com/notary" ts := fullTestServer(t) defer ts.Close() - // uninitialized repo should fail to publish + // uninitialized repo should not fail to publish tempBaseDir, err := ioutil.TempDir("", "notary-tests") require.NoError(t, err) defer os.RemoveAll(tempBaseDir) @@ -1666,14 +1665,31 @@ func TestPublishUninitializedRepo(t *testing.T) { http.DefaultTransport, passphraseRetriever, trustpinning.TrustPinConfig{}) require.NoError(t, err, "error creating repository: %s", err) err = repo.Publish() - require.Error(t, err) + require.NoError(t, err) - // no metadata created - requireRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, false) - requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, false) - requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, false) + // metadata is created + requireRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true) + requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true) + requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true) +} + +// Tnitializing a repo and republishing after should succeed +func TestPublishInitializedRepo(t *testing.T) { + var gun data.GUN = "docker.com/notary" + ts := fullTestServer(t) + defer ts.Close() + + // uninitialized repo should not fail to publish + tempBaseDir, err := ioutil.TempDir("", "notary-tests") + require.NoError(t, err) + defer os.RemoveAll(tempBaseDir) + + // initialized repo should not fail to publish either + repo, err := NewFileCachedNotaryRepository(tempBaseDir, gun, ts.URL, + http.DefaultTransport, passphraseRetriever, trustpinning.TrustPinConfig{}) + require.NoError(t, err, "error creating repository: %s", err) - // now, initialize and republish in the same directory + // now, initialize and publish rootPubKey, err := repo.CryptoService.Create(data.CanonicalRootRole, repo.gun, data.ECDSAKey) require.NoError(t, err, "error generating root key: %s", err) From 170238485bc6dbdc9a3442ff3a03b1fba3fdb70a Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Thu, 2 Mar 2017 15:09:47 +0100 Subject: [PATCH 11/19] Move the decision to init repo based on cached content to its own method Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 50 +++++++++++++++++++---------------------------- client/helpers.go | 22 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/client/client.go b/client/client.go index 5102ca4d9..d5c004830 100644 --- a/client/client.go +++ b/client/client.go @@ -633,23 +633,6 @@ func (r *NotaryRepository) Publish() error { return nil } -func (r *NotaryRepository) isMetaCached() (bool, error) { - for _, role := range data.BaseRoles { - _, err := r.cache.GetSized(role.String(), store.NoSizeLimit) - if err != nil { - if _, ok := err.(store.ErrMetaNotFound); ok { - continue - } - - return false, err - } - - return true, nil - } - - return false, nil -} - // publish pushes the changes in the given changelist to the remote notary-server // Conceptually it performs an operation similar to a `git rebase` func (r *NotaryRepository) publish(cl changelist.Changelist) error { @@ -659,20 +642,8 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { // If the remote is not aware of the repo, then this is being published // for the first time. Try to initialize the repository before publishing. if _, ok := err.(ErrRepositoryNotExist); ok { - metaCached, err := r.isMetaCached() + err := r.initializeFromCache() if err != nil { - logrus.Debugf("Unable to verify on-disk cache: %s", err.Error()) - return err - } - - if metaCached { - err = r.bootstrapRepo() - } else { - err = r.Initialize(nil) - } - if err != nil { - logrus.Debugf("Unable to initialize repository at publish-time: %s", - err.Error()) return err } @@ -884,6 +855,25 @@ func (r *NotaryRepository) bootstrapRepo() error { return nil } +func (r *NotaryRepository) initializeFromCache() error { + metaCached, err := isMetaCached(r.cache) + if err != nil { + logrus.Debugf("Unable to verify on-disk cache: %s", err.Error()) + return err + } + + if metaCached { + err = r.bootstrapRepo() + } else { + err = r.Initialize(nil) + } + if err != nil { + logrus.Debugf("Unable to initialize repository at publish-time: %s", + err.Error()) + return err + } +} + // saveMetadata saves contents of r.tufRepo onto the local disk, creating // signatures as necessary, possibly prompting for passphrases. func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error { diff --git a/client/helpers.go b/client/helpers.go index 5ec0384e2..70a9fbc82 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -7,6 +7,7 @@ import ( "time" "github.com/Sirupsen/logrus" + "github.com/docker/distribution/registry/storage/cache" "github.com/docker/notary/client/changelist" store "github.com/docker/notary/storage" "github.com/docker/notary/tuf" @@ -268,3 +269,24 @@ func serializeCanonicalRole(tufRepo *tuf.Repo, role data.RoleName, extraSigningK return json.Marshal(s) } + +func isMetaCached(cache store.MetadataStore) (bool, error) { + if cache == nil { + return false, nil + } + + for _, role := range data.BaseRoles { + _, err := cache.GetSized(role.String(), store.NoSizeLimit) + if err != nil { + if _, ok := err.(store.ErrMetaNotFound); ok { + continue + } + + return false, err + } + + return true, nil + } + + return false, nil +} From 6a1f1c1f5a528906d486d3d0a21afef98658264f Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Thu, 2 Mar 2017 16:56:47 +0100 Subject: [PATCH 12/19] Move private keys retrieval from a crypto service to a helper function Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 31 ++++++------------------------- client/helpers.go | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/client/client.go b/client/client.go index d5c004830..8e928926f 100644 --- a/client/client.go +++ b/client/client.go @@ -179,31 +179,10 @@ func rootCertKey(gun data.GUN, privKey data.PrivateKey) (data.PublicKey, error) // result is only stored on local disk, not published to the server. To do that, // use r.Publish() eventually. func (r *NotaryRepository) Initialize(rootKeyIDs []string, serverManagedRoles ...data.RoleName) error { - privKeys := make([]data.PrivateKey, 0, len(rootKeyIDs)) - for _, keyID := range rootKeyIDs { - privKey, _, err := r.CryptoService.GetPrivateKey(keyID) - if err != nil { - return err - } - privKeys = append(privKeys, privKey) - } - if len(privKeys) == 0 { - var rootKeyID string - rootKeyList := r.CryptoService.ListKeys(data.CanonicalRootRole) - if len(rootKeyList) == 0 { - rootPublicKey, err := r.CryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey) - if err != nil { - return err - } - rootKeyID = rootPublicKey.ID() - } else { - rootKeyID = rootKeyList[0] - } - privKey, _, err := r.CryptoService.GetPrivateKey(rootKeyID) - if err != nil { - return err - } - privKeys = append(privKeys, privKey) + + privKeys, err := getAllPrivKeys(rootKeyIDs, r.CryptoService) + if err != nil { + return err } // currently we only support server managing timestamps and snapshots, and @@ -872,6 +851,8 @@ func (r *NotaryRepository) initializeFromCache() error { err.Error()) return err } + + return nil } // saveMetadata saves contents of r.tufRepo onto the local disk, creating diff --git a/client/helpers.go b/client/helpers.go index 70a9fbc82..a0511d7db 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -7,11 +7,11 @@ import ( "time" "github.com/Sirupsen/logrus" - "github.com/docker/distribution/registry/storage/cache" "github.com/docker/notary/client/changelist" store "github.com/docker/notary/storage" "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" + "github.com/docker/notary/tuf/signed" "github.com/docker/notary/tuf/utils" ) @@ -290,3 +290,38 @@ func isMetaCached(cache store.MetadataStore) (bool, error) { return false, nil } + +func getAllPrivKeys(rootKeyIDs []string, cryptoService signed.CryptoService) ([]data.PrivateKey, error) { + if cryptoService == nil { + return nil, fmt.Errorf("no crypto service available to get private keys from") + } + + privKeys := make([]data.PrivateKey, 0, len(rootKeyIDs)) + for _, keyID := range rootKeyIDs { + privKey, _, err := cryptoService.GetPrivateKey(keyID) + if err != nil { + return nil, err + } + privKeys = append(privKeys, privKey) + } + if len(privKeys) == 0 { + var rootKeyID string + rootKeyList := cryptoService.ListKeys(data.CanonicalRootRole) + if len(rootKeyList) == 0 { + rootPublicKey, err := cryptoService.Create(data.CanonicalRootRole, "", data.ECDSAKey) + if err != nil { + return nil, err + } + rootKeyID = rootPublicKey.ID() + } else { + rootKeyID = rootKeyList[0] + } + privKey, _, err := cryptoService.GetPrivateKey(rootKeyID) + if err != nil { + return nil, err + } + privKeys = append(privKeys, privKey) + } + + return privKeys, nil +} From 2e8d5ad7ac64cc297566cd4305d5a383c2f32df3 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 3 Mar 2017 18:24:12 +0100 Subject: [PATCH 13/19] Fix test for rotation without explicit init by using a full test server Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client_test.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 7bcc3d2c6..1b91d67c8 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1673,7 +1673,7 @@ func TestPublishUninitializedRepo(t *testing.T) { requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true) } -// Tnitializing a repo and republishing after should succeed +// Initializing a repo and republishing after should succeed func TestPublishInitializedRepo(t *testing.T) { var gun data.GUN = "docker.com/notary" ts := fullTestServer(t) @@ -2647,19 +2647,17 @@ func TestRemoteRotationNoRootKey(t *testing.T) { require.IsType(t, signed.ErrInsufficientSignatures{}, err) } -// The repo is initialized at publish time after -// rotating the key. We should be denied the access -// to metadata by the server when trying to retrieve it. +// The repo should initialize successfully at publish time after +// rotating the key. func TestRemoteRotationNoInit(t *testing.T) { - ts, _, _ := simpleTestServer(t) + ts := fullTestServer(t) defer ts.Close() repo := newBlankRepo(t, ts.URL) defer os.RemoveAll(repo.baseDir) err := repo.RotateKey(data.CanonicalTimestampRole, true, nil) - require.Error(t, err) - require.IsType(t, store.ErrMetaNotFound{}, err) + require.NoError(t, err) } // Rotates the keys. After the rotation, downloading the latest metadata From 9ff8415ed406bd64aa106190cee2ac8f78229b2f Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 3 Mar 2017 18:28:21 +0100 Subject: [PATCH 14/19] Remove the root key cli parameter to 'notary publish' Signed-off-by: Nassim 'Nass' Eddequiouaq --- cmd/notary/tuf.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index c9eda1a7a..3334df373 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -143,7 +143,6 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmd.AddCommand(cmdReset) cmdTUFPublish := cmdTUFPublishTemplate.ToCommand(t.tufPublish) - cmdTUFPublish.Flags().StringVar(&t.rootKey, "rootkey", "", "Root key to initialize the repository with") cmd.AddCommand(cmdTUFPublish) cmd.AddCommand(cmdTUFLookupTemplate.ToCommand(t.tufLookup)) From 73f678c12837e76a72e42553e9c26d8072c55c5d Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 3 Mar 2017 18:55:23 +0100 Subject: [PATCH 15/19] Add comments for metadata lookup function and initializeFromCache Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 2 ++ client/helpers.go | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/client/client.go b/client/client.go index 8e928926f..05da8978c 100644 --- a/client/client.go +++ b/client/client.go @@ -834,6 +834,8 @@ func (r *NotaryRepository) bootstrapRepo() error { return nil } +// initializeFromCache looks for cached metadata to bootstrap from +// and will initialize the repository from scratch otherwise. func (r *NotaryRepository) initializeFromCache() error { metaCached, err := isMetaCached(r.cache) if err != nil { diff --git a/client/helpers.go b/client/helpers.go index a0511d7db..a8d2c1948 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -270,6 +270,8 @@ func serializeCanonicalRole(tufRepo *tuf.Repo, role data.RoleName, extraSigningK return json.Marshal(s) } +// isMetaCached looks for metadata in the cache. For each role, get the associated +// metadata if any. Stop searching if we get an error other than metadata not found. func isMetaCached(cache store.MetadataStore) (bool, error) { if cache == nil { return false, nil @@ -278,10 +280,13 @@ func isMetaCached(cache store.MetadataStore) (bool, error) { for _, role := range data.BaseRoles { _, err := cache.GetSized(role.String(), store.NoSizeLimit) if err != nil { + // no metadata found check if _, ok := err.(store.ErrMetaNotFound); ok { + // try with next role continue } + // error while searching return false, err } From 556e2b49aa41a0df47d48ebd687ca69f356bf694 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 3 Mar 2017 23:29:18 +0100 Subject: [PATCH 16/19] Refactor the metadata retrieval to bootstrap repo from cache or from scratch Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 29 +++++------------------------ client/helpers.go | 26 -------------------------- 2 files changed, 5 insertions(+), 50 deletions(-) diff --git a/client/client.go b/client/client.go index 05da8978c..0c49040d7 100644 --- a/client/client.go +++ b/client/client.go @@ -621,7 +621,11 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { // If the remote is not aware of the repo, then this is being published // for the first time. Try to initialize the repository before publishing. if _, ok := err.(ErrRepositoryNotExist); ok { - err := r.initializeFromCache() + err := r.bootstrapRepo() + if _, ok := err.(store.ErrMetaNotFound); ok { + err = r.Initialize(nil) + } + if err != nil { return err } @@ -834,29 +838,6 @@ func (r *NotaryRepository) bootstrapRepo() error { return nil } -// initializeFromCache looks for cached metadata to bootstrap from -// and will initialize the repository from scratch otherwise. -func (r *NotaryRepository) initializeFromCache() error { - metaCached, err := isMetaCached(r.cache) - if err != nil { - logrus.Debugf("Unable to verify on-disk cache: %s", err.Error()) - return err - } - - if metaCached { - err = r.bootstrapRepo() - } else { - err = r.Initialize(nil) - } - if err != nil { - logrus.Debugf("Unable to initialize repository at publish-time: %s", - err.Error()) - return err - } - - return nil -} - // saveMetadata saves contents of r.tufRepo onto the local disk, creating // signatures as necessary, possibly prompting for passphrases. func (r *NotaryRepository) saveMetadata(ignoreSnapshot bool) error { diff --git a/client/helpers.go b/client/helpers.go index a8d2c1948..ce24c87c5 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -270,32 +270,6 @@ func serializeCanonicalRole(tufRepo *tuf.Repo, role data.RoleName, extraSigningK return json.Marshal(s) } -// isMetaCached looks for metadata in the cache. For each role, get the associated -// metadata if any. Stop searching if we get an error other than metadata not found. -func isMetaCached(cache store.MetadataStore) (bool, error) { - if cache == nil { - return false, nil - } - - for _, role := range data.BaseRoles { - _, err := cache.GetSized(role.String(), store.NoSizeLimit) - if err != nil { - // no metadata found check - if _, ok := err.(store.ErrMetaNotFound); ok { - // try with next role - continue - } - - // error while searching - return false, err - } - - return true, nil - } - - return false, nil -} - func getAllPrivKeys(rootKeyIDs []string, cryptoService signed.CryptoService) ([]data.PrivateKey, error) { if cryptoService == nil { return nil, fmt.Errorf("no crypto service available to get private keys from") From 7e74573ed208298a00b662f54e7788bf1a33da39 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 3 Mar 2017 23:32:50 +0100 Subject: [PATCH 17/19] Remove duplicate test for repo init and publish Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client_test.go | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 1b91d67c8..67cbbbd39 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1673,37 +1673,6 @@ func TestPublishUninitializedRepo(t *testing.T) { requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true) } -// Initializing a repo and republishing after should succeed -func TestPublishInitializedRepo(t *testing.T) { - var gun data.GUN = "docker.com/notary" - ts := fullTestServer(t) - defer ts.Close() - - // uninitialized repo should not fail to publish - tempBaseDir, err := ioutil.TempDir("", "notary-tests") - require.NoError(t, err) - defer os.RemoveAll(tempBaseDir) - - // initialized repo should not fail to publish either - repo, err := NewFileCachedNotaryRepository(tempBaseDir, gun, ts.URL, - http.DefaultTransport, passphraseRetriever, trustpinning.TrustPinConfig{}) - require.NoError(t, err, "error creating repository: %s", err) - - // now, initialize and publish - rootPubKey, err := repo.CryptoService.Create(data.CanonicalRootRole, repo.gun, data.ECDSAKey) - require.NoError(t, err, "error generating root key: %s", err) - - require.NoError(t, repo.Initialize([]string{rootPubKey.ID()})) - - // now metadata is created - requireRepoHasExpectedMetadata(t, repo, data.CanonicalRootRole, true) - requireRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true) - requireRepoHasExpectedMetadata(t, repo, data.CanonicalTargetsRole, true) - - // check we can just call publish again - require.NoError(t, repo.Publish()) -} - // Create a repo, instantiate a notary server, and publish the repo with // some targets to the server, signing all the non-timestamp metadata. // We test this with both an RSA and ECDSA root key From d6ba9f958516755da1a5b68f20b28420282b7238 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Sat, 4 Mar 2017 00:05:31 +0100 Subject: [PATCH 18/19] Add debug logs in publish Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 2 ++ cmd/notary/tuf.go | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/client.go b/client/client.go index 0c49040d7..63e53dea2 100644 --- a/client/client.go +++ b/client/client.go @@ -623,10 +623,12 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { if _, ok := err.(ErrRepositoryNotExist); ok { err := r.bootstrapRepo() if _, ok := err.(store.ErrMetaNotFound); ok { + logrus.Debugf("No metadata found on disk, initializing repository %s from scratch", r.gun.String()) err = r.Initialize(nil) } if err != nil { + logrus.Debugf("Unable to initialize repository at publish-time: %s", err.Error()) return err } diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 3334df373..e3c0c4290 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -142,8 +142,7 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdReset.Flags().BoolVar(&t.resetAll, "all", false, "Reset all changes shown in the status list") cmd.AddCommand(cmdReset) - cmdTUFPublish := cmdTUFPublishTemplate.ToCommand(t.tufPublish) - cmd.AddCommand(cmdTUFPublish) + cmd.AddCommand(cmdTUFPublishTemplate.ToCommand(t.tufPublish)) cmd.AddCommand(cmdTUFLookupTemplate.ToCommand(t.tufLookup)) From dd1ef8b4a92ce4f31cacf1d62036d09bfa588eb6 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Sat, 4 Mar 2017 03:42:34 +0100 Subject: [PATCH 19/19] Improve logs and adjust log-level for repo bootstrap Signed-off-by: Nassim 'Nass' Eddequiouaq --- client/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/client.go b/client/client.go index 63e53dea2..55a5ba4b7 100644 --- a/client/client.go +++ b/client/client.go @@ -623,12 +623,12 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { if _, ok := err.(ErrRepositoryNotExist); ok { err := r.bootstrapRepo() if _, ok := err.(store.ErrMetaNotFound); ok { - logrus.Debugf("No metadata found on disk, initializing repository %s from scratch", r.gun.String()) + logrus.Infof("No TUF data found locally or remotely - initializing repository %s for the first time", r.gun.String()) err = r.Initialize(nil) } if err != nil { - logrus.Debugf("Unable to initialize repository at publish-time: %s", err.Error()) + logrus.WithError(err).Debugf("Unable to load or initialize repository during first publish: %s", err.Error()) return err }