Skip to content

Commit

Permalink
ignore unknown key ids
Browse files Browse the repository at this point in the history
In [TAP-12], we're considering removing the requirement that keyids are
derived using `hexdigest(sha256(cjson(public_key_data)))`. This patch
updates go-tuf to ignore, rather than fail out, if we see a keyid that
wasn't derived with this algorithm. This makes us both forward compatible
to TUF-1.0, and backwards-compatible with go-tuf's hybrid 0.9/1.0
transition.

Test: Added tests that make sure repos can generate new versions of
metadata and preserve unknown keyids, and that clients ignore repos with
random string keyids.

[TAP-12]: https://github.com/theupdateframework/taps/blob/master/tap12.md

Change-Id: I541a67a7beee98078a6e2d9855af184c75ef22e6
  • Loading branch information
erickt committed Dec 23, 2020
1 parent 5eaf689 commit e82b5b6
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 3 deletions.
13 changes: 11 additions & 2 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func (c *Client) Init(rootKeys []*data.Key, threshold int) error {
return err
}

// create a new key database, and add all the public `rootKeys` to it.
c.db = verify.NewDB()
rootKeyIDs := make([]string, 0, len(rootKeys))
for _, key := range rootKeys {
Expand All @@ -114,11 +115,15 @@ func (c *Client) Init(rootKeys []*data.Key, threshold int) error {
}
}
}

// add a mock "root" role that trusts the passed in key ids. These keys
// will be used to verify the `root.json` we just fetched.
role := &data.Role{Threshold: threshold, KeyIDs: rootKeyIDs}
if err := c.db.AddRole("root", role); err != nil {
return err
}

// verify that the new root is valid.
if err := c.decodeRoot(rootJSON); err != nil {
return err
}
Expand Down Expand Up @@ -278,8 +283,12 @@ func (c *Client) getLocalMeta() error {
c.db = verify.NewDB()
for id, k := range root.Keys {
if err := c.db.AddKey(id, k); err != nil {
// FIXME(TUF-0.9) Ignore unknown keyids, which
// can happen during the transition to TUF-1.0.
// TUF is considering in TAP-12 removing the
// requirement that the keyid hash algorithm be derived
// from the public key. So to be forwards compatible,
// we ignore `ErrWrongID` errors.
//
// TAP-12: https://github.com/theupdateframework/taps/blob/master/tap12.md
if _, ok := err.(verify.ErrWrongID); !ok {
return err
}
Expand Down
51 changes: 51 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import (
"testing"
"time"

cjson "github.com/tent/canonical-json-go"
tuf "github.com/theupdateframework/go-tuf"
"github.com/theupdateframework/go-tuf/data"
"github.com/theupdateframework/go-tuf/sign"
"github.com/theupdateframework/go-tuf/util"
"github.com/theupdateframework/go-tuf/verify"
. "gopkg.in/check.v1"
Expand Down Expand Up @@ -897,6 +899,55 @@ func (s *ClientSuite) TestAvailableTarget(c *C) {
c.Assert(err, Equals, ErrNotFound{"/bar.txt"})
}

func (s *ClientSuite) TestUnknownKeyIDs(c *C) {
// get local root.json
meta, err := s.store.GetMeta()
c.Assert(err, IsNil)

rootJSON, ok := meta["root.json"]
c.Assert(ok, Equals, true)

var root struct {
Signed data.Root `json:"signed"`
Signatures []data.Signature `json:signatures"`
}
c.Assert(json.Unmarshal(rootJSON, &root), IsNil)

// update remote root.json to add a new key with an unknown id
key, err := sign.GenerateEd25519Key()
c.Assert(err, IsNil)

root.Signed.Keys["unknown-key-id"] = key.PublicData()

// re-sign the root metadata, then commit it back into the store.
signingKeys, err := s.store.GetSigningKeys("root")
c.Assert(err, IsNil)

signedRoot, err := sign.Marshal(root.Signed, signingKeys...)
c.Assert(err, IsNil)

rootJSON, err = cjson.Marshal(signedRoot)
c.Assert(err, IsNil)

s.store.SetMeta("root.json", rootJSON)
s.store.Commit(false, nil, nil)
s.syncRemote(c)

// FIXME(TUF-0.9) We need this for now because the client still uses
// the TUF-0.9 update workflow, where we decide to update the root
// metadata when we observe a new root through the snapshot.
repo, err := tuf.NewRepo(s.store)
c.Assert(repo.Snapshot(tuf.CompressionTypeNone), IsNil)
c.Assert(repo.Timestamp(), IsNil)
c.Assert(repo.Commit(), IsNil)
s.syncRemote(c)

// Make sure the client can update with the unknown keyid.
client := s.newClient(c)
_, err = client.Update()
c.Assert(err, IsNil)
}

func generateRepoFS(c *C, dir string, files map[string][]byte, consistentSnapshot bool) *tuf.Repo {
repo, err := tuf.NewRepo(tuf.FileSystemStore(dir, nil))
c.Assert(err, IsNil)
Expand Down
10 changes: 9 additions & 1 deletion repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,15 @@ func (r *Repo) db() (*verify.DB, error) {
}
for id, k := range root.Keys {
if err := db.AddKey(id, k); err != nil {
return nil, err
// TUF is considering in TAP-12 removing the
// requirement that the keyid hash algorithm be derived
// from the public key. So to be forwards compatible,
// we ignore `ErrWrongID` errors.
//
// TAP-12: https://github.com/theupdateframework/taps/blob/master/tap12.md
if _, ok := err.(verify.ErrWrongID); !ok {
return nil, err
}
}
}
for name, role := range root.Roles {
Expand Down
70 changes: 70 additions & 0 deletions repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1309,3 +1309,73 @@ func (rs *RepoSuite) TestCustomTargetMetadata(c *C) {
assertCustomMeta("bar.txt", nil)
assertCustomMeta("foo.txt", &custom)
}

func (rs *RepoSuite) TestUnknownKeyIDs(c *C) {
// generate a repo
local := MemoryStore(make(map[string]json.RawMessage), nil)
r, err := NewRepo(local)
c.Assert(err, IsNil)

genKey(c, r, "root")
genKey(c, r, "targets")
genKey(c, r, "snapshot")
genKey(c, r, "timestamp")

// add a new key to the root metadata with an unknown key id.
key, err := sign.GenerateEd25519Key()
c.Assert(err, IsNil)

root, err := r.root()
c.Assert(err, IsNil)
c.Assert(root.Version, Equals, 1)

root.Keys["unknown-key-id"] = key.PublicData()
r.setMeta("root.json", root)

// commit the metadata to the store.
c.Assert(r.AddTargets([]string{}, nil), IsNil)
c.Assert(r.Snapshot(CompressionTypeNone), IsNil)
c.Assert(r.Timestamp(), IsNil)
c.Assert(r.Commit(), IsNil)

// validate that the unknown key id wasn't stripped when written to the
// store.
meta, err := local.GetMeta()
c.Assert(err, IsNil)

rootJSON, ok := meta["root.json"]
c.Assert(ok, Equals, true)

var signedRoot struct {
Signed data.Root `json:"signed"`
Signatures []data.Signature `json:signatures"`
}
c.Assert(json.Unmarshal(rootJSON, &signedRoot), IsNil)
c.Assert(signedRoot.Signed.Version, Equals, 1)

unknownKey, ok := signedRoot.Signed.Keys["unknown-key-id"]
c.Assert(ok, Equals, true)
c.Assert(unknownKey, DeepEquals, key.PublicData())

// make sure if we generate a new root that we preserve the unknown key
// id.
root, err = r.root()

genKey(c, r, "timestamp")
c.Assert(r.Snapshot(CompressionTypeNone), IsNil)
c.Assert(r.Timestamp(), IsNil)
c.Assert(r.Commit(), IsNil)

meta, err = local.GetMeta()
c.Assert(err, IsNil)

rootJSON, ok = meta["root.json"]
c.Assert(ok, Equals, true)

c.Assert(json.Unmarshal(rootJSON, &signedRoot), IsNil)
c.Assert(signedRoot.Signed.Version, Equals, 2)

unknownKey, ok = signedRoot.Signed.Keys["unknown-key-id"]
c.Assert(ok, Equals, true)
c.Assert(unknownKey, DeepEquals, key.PublicData())
}

0 comments on commit e82b5b6

Please sign in to comment.