diff --git a/client/client.go b/client/client.go index 1bf8e5456..a1fbcdde5 100644 --- a/client/client.go +++ b/client/client.go @@ -315,7 +315,7 @@ func addChange(cl *changelist.FileChangelist, c changelist.Change, roles ...stri for _, role := range roles { // Ensure we can only add targets to the CanonicalTargetsRole, // or a Delegation role (which is /something else) - if role != data.CanonicalTargetsRole && !data.IsDelegation(role) { + if role != data.CanonicalTargetsRole && !data.IsDelegation(role) && !data.IsWildDelegation(role) { return data.ErrInvalidRole{ Role: role, Reason: "cannot add targets to this role", diff --git a/client/client_test.go b/client/client_test.go index e3727e8a2..678746af7 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -3041,7 +3041,7 @@ func TestRemoveDelegationChangefileApplicable(t *testing.T) { require.NoError(t, applyTargetsChange(repo.tufRepo, changes[2])) targetRole = repo.tufRepo.Targets[data.CanonicalTargetsRole] - require.Empty(t, targetRole.Signed.Delegations.Roles) + require.Len(t, targetRole.Signed.Delegations.Roles, 1) require.Empty(t, targetRole.Signed.Delegations.Keys) } diff --git a/client/delegations.go b/client/delegations.go index 2c972a824..9b5a40f5a 100644 --- a/client/delegations.go +++ b/client/delegations.go @@ -155,9 +155,11 @@ func (r *NotaryRepository) RemoveDelegationPaths(name string, paths []string) er // RemoveDelegationKeys creates a changelist entry to remove provided keys from an existing delegation. // When this changelist is applied, if the specified keys are the only keys left in the role, // the role itself will be deleted in its entirety. +// It can also delete a key from all delegations under a parent using a name +// with a wildcard at the end. func (r *NotaryRepository) RemoveDelegationKeys(name string, keyIDs []string) error { - if !data.IsDelegation(name) { + if !data.IsDelegation(name) && !data.IsWildDelegation(name) { return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } diff --git a/client/helpers.go b/client/helpers.go index 734fe6107..f188c3a7a 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/http" - "strings" "time" "github.com/Sirupsen/logrus" @@ -41,7 +40,7 @@ func applyChangelist(repo *tuf.Repo, cl changelist.Changelist) error { if err != nil { return err } - isDel := data.IsDelegation(c.Scope()) + isDel := data.IsDelegation(c.Scope()) || data.IsWildDelegation(c.Scope()) switch { case c.Scope() == changelist.ScopeTargets || isDel: err = applyTargetsChange(repo, c) @@ -92,6 +91,10 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error { if err != nil { return err } + if data.IsWildDelegation(c.Scope()) { + return repo.PurgeDelegationKeys(c.Scope(), td.RemoveKeys) + } + delgRole, err := repo.GetDelegationRole(c.Scope()) if err != nil { return err @@ -112,10 +115,6 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error { removeTUFKeyIDs = append(removeTUFKeyIDs, canonicalToTUFID[canonID]) } - // If we specify the only keys left delete the role, else just delete specified keys - if strings.Join(delgRole.ListKeyIDs(), ";") == strings.Join(removeTUFKeyIDs, ";") && len(td.AddKeys) == 0 { - return repo.DeleteDelegation(c.Scope()) - } err = repo.UpdateDelegationKeys(c.Scope(), td.AddKeys, removeTUFKeyIDs, td.NewThreshold) if err != nil { return err diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index 4a3b02cd6..1e7fa73b5 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -32,6 +32,12 @@ var cmdDelegationRemoveTemplate = usageTemplate{ Long: "Remove KeyID(s) from the specified Role delegation in a specific Global Unique Name.", } +var cmdDelegationPurgeKeysTemplate = usageTemplate{ + Use: "purge [ GUN ]", + Short: "Remove KeyID(s) from all delegation roles in the given GUN.", + Long: "Remove KeyID(s) from all delegation roles in the given GUN, for which the signing keys are available. Warnings will be printed for delegations that cannot be updated.", +} + var cmdDelegationAddTemplate = usageTemplate{ Use: "add [ GUN ] [ Role ] ...", Short: "Add a keys to delegation using the provided public key X509 certificates.", @@ -45,12 +51,17 @@ type delegationCommander struct { paths []string allPaths, removeAll, forceYes bool + keyIDs []string } func (d *delegationCommander) GetCommand() *cobra.Command { cmd := cmdDelegationTemplate.ToCommand(nil) cmd.AddCommand(cmdDelegationListTemplate.ToCommand(d.delegationsList)) + cmdPurgeDelgKeys := cmdDelegationPurgeKeysTemplate.ToCommand(d.delegationPurgeKeys) + cmdPurgeDelgKeys.Flags().StringSliceVar(&d.keyIDs, "key", nil, "Delegation key IDs to be removed from the GUN") + cmd.AddCommand(cmdPurgeDelgKeys) + cmdRemDelg := cmdDelegationRemoveTemplate.ToCommand(d.delegationRemove) cmdRemDelg.Flags().StringSliceVar(&d.paths, "paths", nil, "List of paths to remove") cmdRemDelg.Flags().BoolVarP(&d.forceYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") @@ -64,6 +75,53 @@ func (d *delegationCommander) GetCommand() *cobra.Command { return cmd } +func (d *delegationCommander) delegationPurgeKeys(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + cmd.Usage() + return fmt.Errorf("Please provide a single Global Unique Name as an argument to remove") + } + + if len(d.keyIDs) == 0 { + cmd.Usage() + return fmt.Errorf("Please provide at least one key ID to be removed using the --key flag") + } + + gun := args[0] + + config, err := d.configGetter() + if err != nil { + return err + } + + trustPin, err := getTrustPinning(config) + if err != nil { + return err + } + + nRepo, err := notaryclient.NewNotaryRepository( + config.GetString("trust_dir"), + gun, + getRemoteTrustServer(config), + nil, + d.retriever, + trustPin, + ) + if err != nil { + return err + } + + err = nRepo.RemoveDelegationKeys("targets/*", d.keyIDs) + if err != nil { + return fmt.Errorf("failed to remove keys from delegations: %v", err) + } + fmt.Printf( + "Removal of the following keys from all delegations in %s staged for next publish:\n\t- %s\n", + gun, + strings.Join(d.keyIDs, "\n\t- "), + ) + return nil +} + // delegationsList lists all the delegations for a particular GUN func (d *delegationCommander) delegationsList(cmd *cobra.Command, args []string) error { if len(args) != 1 { diff --git a/cmd/notary/delegations_test.go b/cmd/notary/delegations_test.go index 399d9dcfb..d0ce43259 100644 --- a/cmd/notary/delegations_test.go +++ b/cmd/notary/delegations_test.go @@ -14,23 +14,36 @@ import ( "github.com/stretchr/testify/require" ) -var testTrustDir = "trust_dir" - -func setup() *delegationCommander { +func setup(trustDir string) *delegationCommander { return &delegationCommander{ configGetter: func() (*viper.Viper, error) { mainViper := viper.New() - mainViper.Set("trust_dir", testTrustDir) + mainViper.Set("trust_dir", trustDir) return mainViper, nil }, retriever: nil, } } -func TestAddInvalidDelegationName(t *testing.T) { - // Cleanup after test - defer os.RemoveAll(testTrustDir) +func TestPurgeDelegationKeys(t *testing.T) { + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + cmdr := setup(tmpDir) + cmd := cmdr.GetCommand() + err = cmdr.delegationPurgeKeys(cmd, []string{}) + require.Error(t, err) + + err = cmdr.delegationPurgeKeys(cmd, []string{"gun"}) + require.Error(t, err) + + cmdr.keyIDs = []string{"abc"} + err = cmdr.delegationPurgeKeys(cmd, []string{"gun"}) + require.NoError(t, err) +} +func TestAddInvalidDelegationName(t *testing.T) { // Setup certificate tempFile, err := ioutil.TempFile("/tmp", "pemfile") require.NoError(t, err) @@ -41,7 +54,10 @@ func TestAddInvalidDelegationName(t *testing.T) { defer os.Remove(tempFile.Name()) // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to invalid delegation name (should be prefixed by "targets/") err = commander.delegationAdd(commander.GetCommand(), []string{"gun", "INVALID_NAME", tempFile.Name()}) @@ -49,9 +65,6 @@ func TestAddInvalidDelegationName(t *testing.T) { } func TestAddInvalidDelegationCert(t *testing.T) { - // Cleanup after test - defer os.RemoveAll(testTrustDir) - // Setup certificate tempFile, err := ioutil.TempFile("/tmp", "pemfile") require.NoError(t, err) @@ -62,7 +75,10 @@ func TestAddInvalidDelegationCert(t *testing.T) { defer os.Remove(tempFile.Name()) // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to expired cert err = commander.delegationAdd(commander.GetCommand(), []string{"gun", "targets/delegation", tempFile.Name(), "--paths", "path"}) @@ -70,9 +86,6 @@ func TestAddInvalidDelegationCert(t *testing.T) { } func TestAddInvalidShortPubkeyCert(t *testing.T) { - // Cleanup after test - defer os.RemoveAll(testTrustDir) - // Setup certificate tempFile, err := ioutil.TempFile("/tmp", "pemfile") require.NoError(t, err) @@ -83,7 +96,10 @@ func TestAddInvalidShortPubkeyCert(t *testing.T) { defer os.Remove(tempFile.Name()) // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to short RSA key err = commander.delegationAdd(commander.GetCommand(), []string{"gun", "targets/delegation", tempFile.Name(), "--paths", "path"}) @@ -91,53 +107,62 @@ func TestAddInvalidShortPubkeyCert(t *testing.T) { } func TestRemoveInvalidDelegationName(t *testing.T) { - // Cleanup after test - defer os.RemoveAll(testTrustDir) - // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to invalid delegation name (should be prefixed by "targets/") - err := commander.delegationRemove(commander.GetCommand(), []string{"gun", "INVALID_NAME", "fake_key_id1", "fake_key_id2"}) + err = commander.delegationRemove(commander.GetCommand(), []string{"gun", "INVALID_NAME", "fake_key_id1", "fake_key_id2"}) require.Error(t, err) } func TestRemoveAllInvalidDelegationName(t *testing.T) { - // Cleanup after test - defer os.RemoveAll(testTrustDir) - // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to invalid delegation name (should be prefixed by "targets/") - err := commander.delegationRemove(commander.GetCommand(), []string{"gun", "INVALID_NAME"}) + err = commander.delegationRemove(commander.GetCommand(), []string{"gun", "INVALID_NAME"}) require.Error(t, err) } func TestAddInvalidNumArgs(t *testing.T) { // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to invalid number of args (2 instead of 3) - err := commander.delegationAdd(commander.GetCommand(), []string{"not", "enough"}) + err = commander.delegationAdd(commander.GetCommand(), []string{"not", "enough"}) require.Error(t, err) } func TestListInvalidNumArgs(t *testing.T) { // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to invalid number of args (0 instead of 1) - err := commander.delegationsList(commander.GetCommand(), []string{}) + err = commander.delegationsList(commander.GetCommand(), []string{}) require.Error(t, err) } func TestRemoveInvalidNumArgs(t *testing.T) { // Setup commander - commander := setup() + tmpDir, err := ioutil.TempDir("/tmp", "notary-cmd-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + commander := setup(tmpDir) // Should error due to invalid number of args (1 instead of 2) - err := commander.delegationRemove(commander.GetCommand(), []string{"notenough"}) + err = commander.delegationRemove(commander.GetCommand(), []string{"notenough"}) require.Error(t, err) } diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index e5931d043..8c58e9a09 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -555,7 +555,8 @@ func TestClientDelegationsInteraction(t *testing.T) { // list delegations - we should see no delegations output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") require.NoError(t, err) - require.Contains(t, output, "No delegations present in this repository.") + require.NotContains(t, output, keyID) + require.NotContains(t, output, keyID2) // add delegation with multiple certs and multiple paths output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile.Name(), tempFile2.Name(), "--paths", "path1,path2") @@ -1291,3 +1292,77 @@ func TestMain(m *testing.M) { } os.Exit(m.Run()) } + +func TestPurge(t *testing.T) { + setUp(t) + + tempDir := tempDirWithConfig(t, "{}") + defer os.RemoveAll(tempDir) + + server := setupServer() + defer server.Close() + + startTime := time.Now() + endTime := startTime.AddDate(10, 0, 0) + + // Setup certificates + tempFile, err := ioutil.TempFile("", "pemfile") + require.NoError(t, err) + privKey, err := utils.GenerateECDSAKey(rand.Reader) + cert, err := cryptoservice.GenerateCertificate(privKey, "gun", startTime, endTime) + require.NoError(t, err) + _, err = tempFile.Write(utils.CertToPEM(cert)) + require.NoError(t, err) + tempFile.Close() + defer os.Remove(tempFile.Name()) + rawPubBytes, _ := ioutil.ReadFile(tempFile.Name()) + parsedPubKey, _ := utils.ParsePEMPublicKey(rawPubBytes) + keyID, err := utils.CanonicalKeyID(parsedPubKey) + require.NoError(t, err) + + delgName := "targets/delegation" + + _, err = runCommand(t, tempDir, "-s", server.URL, "init", "gun") + require.NoError(t, err) + + _, err = runCommand(t, tempDir, "delegation", "add", "gun", delgName, tempFile.Name(), "--all-paths") + require.NoError(t, err) + + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + require.NoError(t, err) + + // remove targets key and ensure we fail to publish + out, err := runCommand(t, tempDir, "key", "list") + require.NoError(t, err) + lines := splitLines(out) + if len(lines) == 1 && lines[0] == "No signing keys found." { + t.Fail() + } + var targetsKeyID string + for _, line := range lines[2:] { + parts := strings.Fields(line) + if strings.TrimSpace(parts[0]) == data.CanonicalTargetsRole { + targetsKeyID = strings.TrimSpace(parts[2]) + break + } + } + + if targetsKeyID == "" { + t.Fail() + } + + err = os.Remove(filepath.Join(tempDir, notary.PrivDir, notary.NonRootKeysSubdir, "gun", targetsKeyID+".key")) + require.NoError(t, err) + + _, err = runCommand(t, tempDir, "delegation", "purge", "gun", "--key", keyID) + require.NoError(t, err) + + // publish doesn't error because purge only updates the roles we have signing keys for + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + require.NoError(t, err) + + // check the delegation wasn't removed + out, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + require.NoError(t, err) + require.Contains(t, out, delgName) +} diff --git a/docs/advanced_usage.md b/docs/advanced_usage.md index 6c76bedc0..a3e1de39b 100644 --- a/docs/advanced_usage.md +++ b/docs/advanced_usage.md @@ -188,8 +188,8 @@ Forced removal (including all keys and paths) of delegation role targets/release You can remove individual keys and/or paths by passing keys as arguments, and/or paths under the `--paths` flag. Use `--all-paths` to clear all paths for this -role. If you specify all key IDs currently in the delegation role, you will -delete the role entirely. +role. If you specify all key IDs currently in the delegation role, you will be left +with a role that is unusable as it will not have enough valid signatures. To add targets to a specified delegation role, we can use the `notary add` command with the `--roles` flag. diff --git a/tuf/data/roles.go b/tuf/data/roles.go index 742242efb..282377827 100644 --- a/tuf/data/roles.go +++ b/tuf/data/roles.go @@ -96,6 +96,21 @@ func IsBaseRole(role string) bool { return false } +// IsWildDelegation determines if a role represents a valid wildcard delegation +// path, i.e. targets/*, targets/foo/*. +// The wildcard may only appear as the final part of the delegation and must +// be a whole segment, i.e. targets/foo* is not a valid wildcard delegation. +func IsWildDelegation(role string) bool { + if path.Clean(role) != role { + return false + } + base := path.Dir(role) + if !(IsDelegation(base) || base == CanonicalTargetsRole) { + return false + } + return role[len(role)-2:] == "/*" +} + // BaseRole is an internal representation of a root/targets/snapshot/timestamp role, with its public keys included type BaseRole struct { Keys map[string]PublicKey diff --git a/tuf/data/roles_test.go b/tuf/data/roles_test.go index 594060c4e..14dc38ac9 100644 --- a/tuf/data/roles_test.go +++ b/tuf/data/roles_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "fmt" "github.com/stretchr/testify/require" ) @@ -105,44 +106,64 @@ func TestErrInvalidRole(t *testing.T) { } func TestIsDelegation(t *testing.T) { - require.True(t, IsDelegation(path.Join(CanonicalTargetsRole, "level1"))) - require.True(t, IsDelegation( - path.Join(CanonicalTargetsRole, "level1", "level2", "level3"))) - require.True(t, IsDelegation(path.Join(CanonicalTargetsRole, "under_score"))) - require.True(t, IsDelegation(path.Join(CanonicalTargetsRole, "hyphen-hyphen"))) - require.False(t, IsDelegation( - path.Join(CanonicalTargetsRole, strings.Repeat("x", 255-len(CanonicalTargetsRole))))) - - require.False(t, IsDelegation("")) - require.False(t, IsDelegation(CanonicalRootRole)) - require.False(t, IsDelegation(path.Join(CanonicalRootRole, "level1"))) - - require.False(t, IsDelegation(CanonicalTargetsRole)) - require.False(t, IsDelegation(CanonicalTargetsRole+"/")) - require.False(t, IsDelegation(path.Join(CanonicalTargetsRole, "level1")+"/")) - require.False(t, IsDelegation(path.Join(CanonicalTargetsRole, "UpperCase"))) - - require.False(t, IsDelegation( - path.Join(CanonicalTargetsRole, "directory")+"/../../traversal")) - - require.False(t, IsDelegation(CanonicalTargetsRole+"///test/middle/slashes")) - - require.False(t, IsDelegation(CanonicalTargetsRole+"/./././")) - - require.False(t, IsDelegation( - path.Join(" ", CanonicalTargetsRole, "level1"))) - - require.False(t, IsDelegation( - path.Join(" "+CanonicalTargetsRole, "level1"))) - - require.False(t, IsDelegation( - path.Join(CanonicalTargetsRole, "level1"+" "))) + f := require.False + tr := require.True + for val, check := range map[string]func(require.TestingT, bool, ...interface{}){ + // false checks + path.Join(CanonicalTargetsRole, strings.Repeat("x", 255-len(CanonicalTargetsRole))): f, + "": f, + CanonicalRootRole: f, + path.Join(CanonicalRootRole, "level1"): f, + CanonicalTargetsRole: f, + CanonicalTargetsRole + "/": f, + path.Join(CanonicalTargetsRole, "level1") + "/": f, + path.Join(CanonicalTargetsRole, "UpperCase"): f, + path.Join(CanonicalTargetsRole, "directory") + "/../../traversal": f, + CanonicalTargetsRole + "///test/middle/slashes": f, + CanonicalTargetsRole + "/./././": f, + path.Join(" ", CanonicalTargetsRole, "level1"): f, + path.Join(" "+CanonicalTargetsRole, "level1"): f, + path.Join(CanonicalTargetsRole, "level1"+" "): f, + path.Join(CanonicalTargetsRole, "white space"+"level2"): f, + path.Join(CanonicalTargetsRole, strings.Repeat("x", 256-len(CanonicalTargetsRole))): f, + + // true checks + path.Join(CanonicalTargetsRole, "level1"): tr, + path.Join(CanonicalTargetsRole, "level1", "level2", "level3"): tr, + path.Join(CanonicalTargetsRole, "under_score"): tr, + path.Join(CanonicalTargetsRole, "hyphen-hyphen"): tr, + } { + check(t, IsDelegation(val)) + } - require.False(t, IsDelegation( - path.Join(CanonicalTargetsRole, "white space"+"level2"))) +} - require.False(t, IsDelegation( - path.Join(CanonicalTargetsRole, strings.Repeat("x", 256-len(CanonicalTargetsRole))))) +func TestIsWildDelegation(t *testing.T) { + f := require.False + tr := require.True + for val, check := range map[string]func(require.TestingT, bool, ...interface{}){ + // false checks + CanonicalRootRole: f, + CanonicalTargetsRole: f, + CanonicalSnapshotRole: f, + CanonicalTimestampRole: f, + "foo": f, + "foo/*": f, + path.Join(CanonicalRootRole, "*"): f, + path.Join(CanonicalSnapshotRole, "*"): f, + path.Join(CanonicalTimestampRole, "*"): f, + path.Join(CanonicalTargetsRole, "*", "foo"): f, + path.Join(CanonicalTargetsRole, "*", "*"): f, + fmt.Sprintf("%s//*", CanonicalTargetsRole): f, + fmt.Sprintf("%s/*//", CanonicalTargetsRole): f, + fmt.Sprintf("%s/*/", CanonicalTargetsRole): f, + + // true checks + path.Join(CanonicalTargetsRole, "*"): tr, + path.Join(CanonicalTargetsRole, "foo", "*"): tr, + } { + check(t, IsWildDelegation(val)) + } } func TestValidRoleFunction(t *testing.T) { diff --git a/tuf/tuf.go b/tuf/tuf.go index 8f383852d..34621b37f 100644 --- a/tuf/tuf.go +++ b/tuf/tuf.go @@ -325,17 +325,16 @@ func delegationUpdateVisitor(roleName string, addKeys data.KeyList, removeKeys, break } } - // We didn't find the role earlier, so create it only if we have keys to add + // We didn't find the role earlier, so create it. + if addKeys == nil { + addKeys = data.KeyList{} // initialize to empty list if necessary so calling .IDs() below won't panic + } if delgRole == nil { - if len(addKeys) > 0 { - delgRole, err = data.NewRole(roleName, newThreshold, addKeys.IDs(), addPaths) - if err != nil { - return err - } - } else { - // If we can't find the role and didn't specify keys to add, this is an error - return data.ErrInvalidRole{Role: roleName, Reason: "cannot create new delegation without keys"} + delgRole, err = data.NewRole(roleName, newThreshold, addKeys.IDs(), addPaths) + if err != nil { + return err } + } // Add the key IDs to the role and the keys themselves to the parent for _, k := range addKeys { @@ -345,7 +344,7 @@ func delegationUpdateVisitor(roleName string, addKeys data.KeyList, removeKeys, } // Make sure we have a valid role still if len(delgRole.KeyIDs) < delgRole.Threshold { - return data.ErrInvalidRole{Role: roleName, Reason: "insufficient keys to meet threshold"} + logrus.Warnf("role %s has fewer keys than its threshold of %d; it will not be usable until keys are added to it", delgRole.Name, delgRole.Threshold) } // NOTE: this closure CANNOT error after this point, as we've committed to editing the SignedTargets metadata in the repo object. // Any errors related to updating this delegation must occur before this point. @@ -392,11 +391,76 @@ func (tr *Repo) UpdateDelegationKeys(roleName string, addKeys data.KeyList, remo // Walk to the parent of this delegation, since that is where its role metadata exists // We do not have to verify that the walker reached its desired role in this scenario // since we've already done another walk to the parent role in VerifyCanSign, and potentially made a targets file - err := tr.WalkTargets("", parent, delegationUpdateVisitor(roleName, addKeys, removeKeys, []string{}, []string{}, false, newThreshold)) - if err != nil { - return err + return tr.WalkTargets("", parent, delegationUpdateVisitor(roleName, addKeys, removeKeys, []string{}, []string{}, false, newThreshold)) +} + +// PurgeDelegationKeys removes the provided canonical key IDs from all delegations +// present in the subtree rooted at role. The role argument must be provided in a wildcard +// format, i.e. targets/* would remove the key from all delegations in the repo +func (tr *Repo) PurgeDelegationKeys(role string, removeKeys []string) error { + if !data.IsWildDelegation(role) { + return data.ErrInvalidRole{ + Role: role, + Reason: "only wildcard roles can be used in a purge", + } } - return nil + + removeIDs := make(map[string]struct{}) + for _, id := range removeKeys { + removeIDs[id] = struct{}{} + } + + start := path.Dir(role) + tufIDToCanon := make(map[string]string) + + purgeKeys := func(tgt *data.SignedTargets, validRole data.DelegationRole) interface{} { + var ( + deleteCandidates []string + err error + ) + for id, key := range tgt.Signed.Delegations.Keys { + var ( + canonID string + ok bool + ) + if canonID, ok = tufIDToCanon[id]; !ok { + canonID, err = utils.CanonicalKeyID(key) + if err != nil { + return err + } + tufIDToCanon[id] = canonID + } + if _, ok := removeIDs[canonID]; ok { + deleteCandidates = append(deleteCandidates, id) + } + } + if len(deleteCandidates) == 0 { + // none of the interesting keys were present. We're done with this role + return nil + } + // now we know there are changes, check if we'll be able to sign them in + if err := tr.VerifyCanSign(validRole.Name); err != nil { + logrus.Warnf( + "role %s contains keys being purged but you do not have the necessary keys present to sign it; keys will not be purged from %s or its immediate children", + validRole.Name, + validRole.Name, + ) + return nil + } + // we know we can sign in the changes, delete the keys + for _, id := range deleteCandidates { + delete(tgt.Signed.Delegations.Keys, id) + } + // delete candidate keys from all roles. + for _, role := range tgt.Signed.Delegations.Roles { + role.RemoveKeys(deleteCandidates) + if len(role.KeyIDs) < role.Threshold { + logrus.Warnf("role %s has fewer keys than its threshold of %d; it will not be usable until keys are added to it", role.Name, role.Threshold) + } + } + return nil + } + return tr.WalkTargets("", start, purgeKeys) } // UpdateDelegationPaths updates the appropriate delegation's paths. diff --git a/tuf/tuf_test.go b/tuf/tuf_test.go index 54d939d40..c40571ed7 100644 --- a/tuf/tuf_test.go +++ b/tuf/tuf_test.go @@ -166,6 +166,148 @@ func TestUpdateDelegations(t *testing.T) { require.False(t, ok, "no empty targets file should be created for deepest delegation") } +func TestPurgeDelegationsKeyFromTop(t *testing.T) { + ed25519 := signed.NewEd25519() + repo := initRepo(t, ed25519) + + vetinari := path.Join(data.CanonicalTargetsRole, "vetinari") + sybil := path.Join(data.CanonicalTargetsRole, "sybil") + vimes := path.Join(data.CanonicalTargetsRole, "vimes") + carrot := path.Join(vimes, "carrot") + targetsWild := path.Join(data.CanonicalTargetsRole, "*") + + // create 2 keys, we'll purge one of them + testKey1, err := ed25519.Create(vetinari, testGUN, data.ED25519Key) + require.NoError(t, err) + testKey2, err := ed25519.Create(vetinari, testGUN, data.ED25519Key) + require.NoError(t, err) + + // create some delegations + err = repo.UpdateDelegationKeys(vetinari, []data.PublicKey{testKey1, testKey2}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(vetinari, []string{""}, []string{}, false) + require.NoError(t, err) + + err = repo.UpdateDelegationKeys(sybil, []data.PublicKey{testKey1}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(sybil, []string{""}, []string{}, false) + require.NoError(t, err) + + err = repo.UpdateDelegationKeys(vimes, []data.PublicKey{testKey2}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(vimes, []string{""}, []string{}, false) + require.NoError(t, err) + + err = repo.UpdateDelegationKeys(carrot, []data.PublicKey{testKey1}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(carrot, []string{""}, []string{}, false) + require.NoError(t, err) + + id1, err := utils.CanonicalKeyID(testKey1) + require.NoError(t, err) + err = repo.PurgeDelegationKeys(targetsWild, []string{id1}) + require.NoError(t, err) + + role, err := repo.GetDelegationRole(vetinari) + require.NoError(t, err) + require.Len(t, role.Keys, 1) + _, ok := role.Keys[testKey2.ID()] + require.True(t, ok) + + role, err = repo.GetDelegationRole(sybil) + require.NoError(t, err) + require.Len(t, role.Keys, 0) + + role, err = repo.GetDelegationRole(vimes) + require.NoError(t, err) + require.Len(t, role.Keys, 1) + _, ok = role.Keys[testKey2.ID()] + require.True(t, ok) + + role, err = repo.GetDelegationRole(carrot) + require.NoError(t, err) + require.Len(t, role.Keys, 0) + + // we know id1 was successfully purged, try purging again and make sure it doesn't error + err = repo.PurgeDelegationKeys(targetsWild, []string{id1}) + require.NoError(t, err) +} + +func TestPurgeDelegationsKeyFromDeep(t *testing.T) { + ed25519 := signed.NewEd25519() + repo := initRepo(t, ed25519) + + vetinari := path.Join(data.CanonicalTargetsRole, "vetinari") + sybil := path.Join(data.CanonicalTargetsRole, "sybil") + vimes := path.Join(data.CanonicalTargetsRole, "vimes") + carrot := path.Join(vimes, "carrot") + vimesWild := path.Join(vimes, "*") + + // create 2 keys, we'll purge one of them + testKey1, err := ed25519.Create(vetinari, testGUN, data.ED25519Key) + require.NoError(t, err) + testKey2, err := ed25519.Create(vetinari, testGUN, data.ED25519Key) + require.NoError(t, err) + + // create some delegations + err = repo.UpdateDelegationKeys(vetinari, []data.PublicKey{testKey1, testKey2}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(vetinari, []string{""}, []string{}, false) + require.NoError(t, err) + + err = repo.UpdateDelegationKeys(sybil, []data.PublicKey{testKey1}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(sybil, []string{""}, []string{}, false) + require.NoError(t, err) + + err = repo.UpdateDelegationKeys(vimes, []data.PublicKey{testKey2}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(vimes, []string{""}, []string{}, false) + require.NoError(t, err) + + err = repo.UpdateDelegationKeys(carrot, []data.PublicKey{testKey1}, []string{}, 1) + require.NoError(t, err) + err = repo.UpdateDelegationPaths(carrot, []string{""}, []string{}, false) + require.NoError(t, err) + + id1, err := utils.CanonicalKeyID(testKey1) + require.NoError(t, err) + err = repo.PurgeDelegationKeys(vimesWild, []string{id1}) + require.NoError(t, err) + + role, err := repo.GetDelegationRole(vetinari) + require.NoError(t, err) + require.Len(t, role.Keys, 2) + _, ok := role.Keys[testKey1.ID()] + require.True(t, ok) + _, ok = role.Keys[testKey2.ID()] + require.True(t, ok) + + role, err = repo.GetDelegationRole(sybil) + require.NoError(t, err) + require.Len(t, role.Keys, 1) + _, ok = role.Keys[testKey1.ID()] + require.True(t, ok) + + role, err = repo.GetDelegationRole(vimes) + require.NoError(t, err) + require.Len(t, role.Keys, 1) + _, ok = role.Keys[testKey2.ID()] + require.True(t, ok) + + role, err = repo.GetDelegationRole(carrot) + require.NoError(t, err) + require.Len(t, role.Keys, 0) +} + +func TestPurgeDelegationsKeyBadWildRole(t *testing.T) { + ed25519 := signed.NewEd25519() + repo := initRepo(t, ed25519) + + err := repo.PurgeDelegationKeys("targets/foo", nil) + require.Error(t, err) + require.IsType(t, data.ErrInvalidRole{}, err) +} func TestUpdateDelegationsParentMissing(t *testing.T) { ed25519 := signed.NewEd25519() repo := initRepo(t, ed25519) @@ -262,8 +404,7 @@ func TestUpdateDelegationsNotEnoughKeys(t *testing.T) { require.NoError(t, err) err = repo.UpdateDelegationKeys("targets/role", []data.PublicKey{roleKey}, []string{}, 2) - require.Error(t, err) - require.IsType(t, data.ErrInvalidRole{}, err) + require.NoError(t, err) // no delegation metadata created for failed delegation _, ok := repo.Targets["targets/role"]