From 545f98e31de0619bb544cf26d988a4fb53b4e1cb Mon Sep 17 00:00:00 2001 From: Zack Newman Date: Wed, 23 Mar 2022 13:23:17 -0400 Subject: [PATCH] Add Reason to ErrInvalidKeys (#237) Leads to better error messages throughout. Also improve repo_test coverage to exercise all of the ErrInvalidKeys paths. Signed-off-by: Zachary Newman --- errors.go | 5 +++-- repo.go | 23 ++++++++++++++--------- repo_test.go | 33 +++++++++++++++++++++++++++++---- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/errors.go b/errors.go index 33d67c4b..1a8ab9ff 100644 --- a/errors.go +++ b/errors.go @@ -46,11 +46,12 @@ func (e ErrInsufficientSignatures) Error() string { } type ErrInvalidRole struct { - Role string + Role string + Reason string } func (e ErrInvalidRole) Error() string { - return fmt.Sprintf("tuf: invalid role %s", e.Role) + return fmt.Sprintf("tuf: invalid role %s: %s", e.Role, e.Reason) } type ErrInvalidExpires struct { diff --git a/repo.go b/repo.go index cb6922ba..53a65a72 100644 --- a/repo.go +++ b/repo.go @@ -152,13 +152,18 @@ func (r *Repo) RootVersion() (int, error) { } func (r *Repo) GetThreshold(keyRole string) (int, error) { + if !roles.IsTopLevelRole(keyRole) { + // Delegations are not currently supported, so return an error if this is not a + // top-level metadata file. + return -1, ErrInvalidRole{keyRole, "only thresholds for top-level roles supported"} + } root, err := r.root() if err != nil { return -1, err } role, ok := root.Roles[keyRole] if !ok { - return -1, ErrInvalidRole{keyRole} + return -1, ErrInvalidRole{keyRole, "role missing from root metadata"} } return role.Threshold, nil @@ -168,7 +173,7 @@ func (r *Repo) SetThreshold(keyRole string, t int) error { if !roles.IsTopLevelRole(keyRole) { // Delegations are not currently supported, so return an error if this is not a // top-level metadata file. - return ErrInvalidRole{keyRole} + return ErrInvalidRole{keyRole, "only thresholds for top-level roles supported"} } root, err := r.root() if err != nil { @@ -176,7 +181,7 @@ func (r *Repo) SetThreshold(keyRole string, t int) error { } role, ok := root.Roles[keyRole] if !ok { - return ErrInvalidRole{keyRole} + return ErrInvalidRole{keyRole, "role missing from root metadata"} } if role.Threshold == t { // Change was a no-op. @@ -283,7 +288,7 @@ func (r *Repo) timestamp() (*data.Timestamp, error) { func (r *Repo) ChangePassphrase(keyRole string) error { if !roles.IsTopLevelRole(keyRole) { - return ErrInvalidRole{keyRole} + return ErrInvalidRole{keyRole, "only support passphrases for top-level roles"} } if p, ok := r.local.(PassphraseChanger); ok { @@ -316,7 +321,7 @@ func (r *Repo) AddPrivateKey(role string, signer keys.Signer) error { func (r *Repo) AddPrivateKeyWithExpires(keyRole string, signer keys.Signer, expires time.Time) error { if !roles.IsTopLevelRole(keyRole) { - return ErrInvalidRole{keyRole} + return ErrInvalidRole{keyRole, "only support adding keys for top-level roles"} } if !validExpires(expires) { @@ -414,7 +419,7 @@ func (r *Repo) RevokeKey(role, id string) error { func (r *Repo) RevokeKeyWithExpires(keyRole, id string, expires time.Time) error { if !roles.IsTopLevelRole(keyRole) { - return ErrInvalidRole{keyRole} + return ErrInvalidRole{keyRole, "only revocations for top-level roles supported"} } if !validExpires(expires) { @@ -517,7 +522,7 @@ func (r *Repo) setTopLevelMeta(roleFilename string, meta interface{}) error { func (r *Repo) Sign(roleFilename string) error { role := strings.TrimSuffix(roleFilename, ".json") if !roles.IsTopLevelRole(role) { - return ErrInvalidRole{role} + return ErrInvalidRole{role, "only signing top-level metadata supported"} } s, err := r.SignedMeta(roleFilename) @@ -553,7 +558,7 @@ func (r *Repo) Sign(roleFilename string) error { func (r *Repo) AddOrUpdateSignature(roleFilename string, signature data.Signature) error { role := strings.TrimSuffix(roleFilename, ".json") if !roles.IsTopLevelRole(role) { - return ErrInvalidRole{role} + return ErrInvalidRole{role, "only signing top-level metadata supported"} } // Check key ID is in valid for the role. @@ -563,7 +568,7 @@ func (r *Repo) AddOrUpdateSignature(roleFilename string, signature data.Signatur } roleData := db.GetRole(role) if roleData == nil { - return ErrInvalidRole{role} + return ErrInvalidRole{role, "role missing from top-level keys"} } if !roleData.ValidKey(signature.KeyID) { return verify.ErrInvalidKey diff --git a/repo_test.go b/repo_test.go index eb9356ef..33643735 100644 --- a/repo_test.go +++ b/repo_test.go @@ -184,7 +184,7 @@ func (rs *RepoSuite) TestGenKey(c *C) { // generate a key for an unknown role _, err = r.GenKey("foo") - c.Assert(err, Equals, ErrInvalidRole{"foo"}) + c.Assert(err, Equals, ErrInvalidRole{"foo", "only support adding keys for top-level roles"}) // generate a root key ids := genKey(c, r, "root") @@ -346,7 +346,7 @@ func (rs *RepoSuite) TestAddPrivateKey(c *C) { signer, err := keys.GenerateEd25519Key() c.Assert(err, IsNil) err = r.AddPrivateKey("foo", signer) - c.Assert(err, Equals, ErrInvalidRole{"foo"}) + c.Assert(err, Equals, ErrInvalidRole{"foo", "only support adding keys for top-level roles"}) // add a root key ids := addPrivateKey(c, r, "root", signer) @@ -511,7 +511,7 @@ func (rs *RepoSuite) TestRevokeKey(c *C) { c.Assert(err, IsNil) // revoking a key for an unknown role returns ErrInvalidRole - c.Assert(r.RevokeKey("foo", ""), DeepEquals, ErrInvalidRole{"foo"}) + c.Assert(r.RevokeKey("foo", ""), DeepEquals, ErrInvalidRole{"foo", "only revocations for top-level roles supported"}) // revoking a key which doesn't exist returns ErrKeyNotFound c.Assert(r.RevokeKey("root", "nonexistent"), DeepEquals, ErrKeyNotFound{"root", "nonexistent"}) @@ -632,6 +632,8 @@ func (rs *RepoSuite) TestSign(c *C) { r, err := NewRepo(local) c.Assert(err, IsNil) + c.Assert(r.Sign("foo.json"), Equals, ErrInvalidRole{"foo", "only signing top-level metadata supported"}) + // signing with no keys returns ErrInsufficientKeys c.Assert(r.Sign("root.json"), Equals, ErrInsufficientKeys{"root.json"}) @@ -676,6 +678,9 @@ func (rs *RepoSuite) TestSign(c *C) { c.Assert(local.SaveSigner("root", newKey), IsNil) c.Assert(r.Sign("root.json"), IsNil) checkSigIDs(append(signer.PublicData().IDs(), newKey.PublicData().IDs()...)...) + + // attempt to sign missing metadata + c.Assert(r.Sign("targets.json"), Equals, ErrMissingMetadata{"targets.json"}) } func (rs *RepoSuite) TestCommit(c *C) { @@ -1386,6 +1391,12 @@ func (rs *RepoSuite) TestKeyPersistence(c *C) { tmp = newTmpDir(c) store = FileSystemStore(tmp.path, testPassphraseFunc) + // 1.5. Changing passphrase only works for top-level roles. + r, err := NewRepo(store) + c.Assert(err, IsNil) + + c.Assert(r.ChangePassphrase("foo"), DeepEquals, ErrInvalidRole{"foo", "only support passphrases for top-level roles"}) + // 2. Test changing the passphrase when the keys file does not exist - should FAIL c.Assert(store.(PassphraseChanger).ChangePassphrase("root"), NotNil) @@ -1598,6 +1609,11 @@ func (rs *RepoSuite) TestThreshold(c *C) { r, err := NewRepo(local) c.Assert(err, IsNil) + _, err = r.GetThreshold("root") + c.Assert(err, DeepEquals, ErrInvalidRole{"root", "role missing from root metadata"}) + err = r.SetThreshold("root", 2) + c.Assert(err, DeepEquals, ErrInvalidRole{"root", "role missing from root metadata"}) + // Add one key to each role genKey(c, r, "root") genKey(c, r, "targets") @@ -1607,6 +1623,11 @@ func (rs *RepoSuite) TestThreshold(c *C) { c.Assert(err, IsNil) c.Assert(t, Equals, 1) + _, err = r.GetThreshold("foo") + c.Assert(err, DeepEquals, ErrInvalidRole{"foo", "only thresholds for top-level roles supported"}) + err = r.SetThreshold("foo", 2) + c.Assert(err, DeepEquals, ErrInvalidRole{"foo", "only thresholds for top-level roles supported"}) + // commit the metadata to the store. c.Assert(r.AddTargets([]string{}, nil), IsNil) c.Assert(r.Snapshot(), IsNil) @@ -1727,6 +1748,10 @@ func (rs *RepoSuite) TestBadAddOrUpdateSignatures(c *C) { // don't use consistent snapshots to make the checks simpler c.Assert(r.Init(false), IsNil) + c.Assert(r.AddOrUpdateSignature("targets.json", data.Signature{ + KeyID: "foo", + Signature: nil}), Equals, ErrInvalidRole{"targets", "role missing from top-level keys"}) + // generate root key offline and add as a verification key rootKey, err := keys.GenerateEd25519Key() c.Assert(err, IsNil) @@ -1749,7 +1774,7 @@ func (rs *RepoSuite) TestBadAddOrUpdateSignatures(c *C) { for _, id := range rootKey.PublicData().IDs() { c.Assert(r.AddOrUpdateSignature("invalid_root.json", data.Signature{ KeyID: id, - Signature: rootSig}), Equals, ErrInvalidRole{"invalid_root"}) + Signature: rootSig}), Equals, ErrInvalidRole{"invalid_root", "only signing top-level metadata supported"}) } // add a root signature with an key ID that is for the targets role