Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command to purge all keys from all delegations below a starting point #855

Merged
merged 3 commits into from
Jul 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <CanonicalTargetsRole>/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",
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 3 additions & 1 deletion client/delegations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
}

Expand Down
11 changes: 5 additions & 6 deletions client/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"fmt"
"net/http"
"strings"
"time"

"github.com/Sirupsen/logrus"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -92,6 +91,10 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error {
if err != nil {
return err
}
if data.IsWildDelegation(c.Scope()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never getting exercised, which is weird because cmd/notary/integration_test.go's TestPurge should exercise this bit of code.

In applyChangelist, where we switch based on whether the scope is a delegation (L43) I think we also have to change that line to be isDel := data.IsDelegation(c.Scope()) || data.IsWildDelegation(c.Scope())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, you're absolutely right, and that's making me wonder why the test in cmd/notary/delegations_test.go is even passing.

Going to add some more checks to that test to try and make it fail, then will update this and see if it passes again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see now. I'm calling the purge command but not publish so it's not actually applying the change. I must have got sidetracked half way through the test and forgotten to come back to it.

return repo.PurgeDelegationKeys(c.Scope(), td.RemoveKeys)
}

delgRole, err := repo.GetDelegationRole(c.Scope())
if err != nil {
return err
Expand All @@ -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
Expand Down
58 changes: 58 additions & 0 deletions cmd/notary/delegations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ] <X509 file path 1> ...",
Short: "Add a keys to delegation using the provided public key X509 certificates.",
Expand All @@ -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)")
Expand All @@ -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 {
Expand Down
89 changes: 57 additions & 32 deletions cmd/notary/delegations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o/ Yay injecting all the things!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also using temp dirs rather than ./trust_dir

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)
Expand All @@ -41,17 +54,17 @@ 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()})
require.Error(t, err)
}

func TestAddInvalidDelegationCert(t *testing.T) {
// Cleanup after test
defer os.RemoveAll(testTrustDir)

// Setup certificate
tempFile, err := ioutil.TempFile("/tmp", "pemfile")
require.NoError(t, err)
Expand All @@ -62,17 +75,17 @@ 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"})
require.Error(t, err)
}

func TestAddInvalidShortPubkeyCert(t *testing.T) {
// Cleanup after test
defer os.RemoveAll(testTrustDir)

// Setup certificate
tempFile, err := ioutil.TempFile("/tmp", "pemfile")
require.NoError(t, err)
Expand All @@ -83,61 +96,73 @@ 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"})
require.Error(t, err)
}

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)
}

Expand Down
Loading