Skip to content

Commit

Permalink
Provide a more useful error message when a delegation role to be witn…
Browse files Browse the repository at this point in the history
…essed is invalid

Signed-off-by: Ying Li <ying.li@docker.com>
  • Loading branch information
cyli committed Sep 26, 2016
1 parent 84bb4ae commit 7772deb
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
16 changes: 14 additions & 2 deletions client/witness.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package client

import (
"path/filepath"

"github.com/docker/notary/client/changelist"
"github.com/docker/notary/tuf"
"github.com/docker/notary/tuf/data"
"path/filepath"
)

// Witness creates change objects to witness (i.e. re-sign) the given
Expand Down Expand Up @@ -41,7 +42,18 @@ func witnessTargets(repo *tuf.Repo, invalid *tuf.Repo, role string) error {
r.Dirty = true
return nil
}
if invalid != nil {

if roleObj, err := repo.GetDelegationRole(role); err == nil && invalid != nil {
// A role with a threshold > len(keys) is technically invalid, but we let it build in the builder because
// we want to be able to download the role (which may still have targets on it), add more keys, and then
// witness the role, thus bringing it back to valid. However, if no keys have been added before witnessing,
// then it is still an invalid role, and can't be witnessed because nothing can bring it back to valid.
if roleObj.Threshold > len(roleObj.Keys) {
return data.ErrInvalidRole{
Role: role,
Reason: "role does not specify enough valid signing keys to meet its required threshold",
}
}
if r, ok := invalid.Targets[role]; ok {
// role is recognized but invalid, move to valid data and mark for re-signing
repo.Targets[role] = r
Expand Down
8 changes: 8 additions & 0 deletions cmd/notary/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1471,6 +1471,8 @@ func TestPurgeSingleKey(t *testing.T) {
// 11. witness an invalid role and check for error on publish
// 12. check non-targets base roles all fail
// 13. test auto-publish functionality
// 14. remove all keys from the delegation and publish
// 15. witnessing the delegation should now fail
func TestWitness(t *testing.T) {
setUp(t)

Expand Down Expand Up @@ -1637,6 +1639,12 @@ func TestWitness(t *testing.T) {
require.NoError(t, err)
require.Contains(t, output, targetName)
require.Contains(t, output, targetHash)

_, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "remove", "-p", "gun", delgName, keyID, keyID2)
require.NoError(t, err)
_, err = runCommand(t, tempDir, "-s", server.URL, "witness", "-p", "gun", delgName)
require.Error(t, err)
require.Contains(t, err.Error(), "role does not specify enough valid signing keys to meet its required threshold")
}

func generateCertPrivKeyPair(t *testing.T, gun, keyAlgorithm string) (*x509.Certificate, data.PrivateKey, string) {
Expand Down

0 comments on commit 7772deb

Please sign in to comment.