diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6fb2dcd52..a805655ac 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -59,9 +59,11 @@ jobs: needs: get-go-versions steps: - uses: actions/setup-go@268d8c0ca0432bb2cf416faae41297df9d262d7f + with: + go-version: ${{ matrix.go-version }} - uses: actions/checkout@v3 - name: golangci-lint uses: golangci/golangci-lint-action@537aa1903e5d359d0b27dbc19ddd22c5087f3fbc with: - version: v1.45.2 + version: v1.49.0 args: --timeout 3m diff --git a/.golangci.yml b/.golangci.yml index 4d860521b..6e8bf3c86 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,9 @@ +run: + # Lint using Go 1.17, since some linters are disabled by default for Go 1.18 + # until generics are supported. + # See https://github.com/golangci/golangci-lint/issues/2649 + go: '1.17' + linters: disable-all: true enable: @@ -5,7 +11,5 @@ linters: - gofmt - govet - gosimple - - structcheck - - varcheck - unused - typecheck diff --git a/client/client.go b/client/client.go index 188d8a8a8..17ddc9805 100644 --- a/client/client.go +++ b/client/client.go @@ -5,7 +5,6 @@ import ( "encoding/hex" "encoding/json" "io" - "io/ioutil" "github.com/theupdateframework/go-tuf/data" "github.com/theupdateframework/go-tuf/util" @@ -551,7 +550,7 @@ func (c *Client) downloadMetaUnsafe(name string, maxMetaSize int64) ([]byte, err // although the size has been checked above, use a LimitReader in case // the reported size is inaccurate, or size is -1 which indicates an // unknown length - return ioutil.ReadAll(io.LimitReader(r, maxMetaSize)) + return io.ReadAll(io.LimitReader(r, maxMetaSize)) } // remoteGetFunc is the type of function the download method uses to download @@ -622,7 +621,7 @@ func (c *Client) downloadMeta(name string, version int64, m data.FileMeta) ([]by stream = r } - return ioutil.ReadAll(stream) + return io.ReadAll(stream) } func (c *Client) downloadMetaFromSnapshot(name string, m data.SnapshotFileMeta) ([]byte, error) { @@ -673,17 +672,6 @@ func (c *Client) downloadMetaFromTimestamp(name string, m data.TimestampFileMeta return b, nil } -// decodeRoot decodes and verifies root metadata. -func (c *Client) decodeRoot(b json.RawMessage) error { - root := &data.Root{} - if err := c.db.Unmarshal(b, root, "root", c.rootVer); err != nil { - return ErrDecodeFailed{"root.json", err} - } - c.rootVer = root.Version - c.consistentSnapshot = root.ConsistentSnapshot - return nil -} - // decodeSnapshot decodes and verifies snapshot metadata, and returns the new // root and targets file meta. func (c *Client) decodeSnapshot(b json.RawMessage) (data.SnapshotFiles, error) { @@ -790,36 +778,6 @@ func (c *Client) localMetaFromSnapshot(name string, m data.SnapshotFileMeta) (js return b, err == nil } -// hasTargetsMeta checks whether local metadata has the given snapshot meta -//lint:ignore U1000 unused -func (c *Client) hasTargetsMeta(m data.SnapshotFileMeta) bool { - b, ok := c.localMeta["targets.json"] - if !ok { - return false - } - meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) - if err != nil { - return false - } - err = util.SnapshotFileMetaEqual(meta, m) - return err == nil -} - -// hasSnapshotMeta checks whether local metadata has the given meta -//lint:ignore U1000 unused -func (c *Client) hasMetaFromTimestamp(name string, m data.TimestampFileMeta) bool { - b, ok := c.localMeta[name] - if !ok { - return false - } - meta, err := util.GenerateTimestampFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) - if err != nil { - return false - } - err = util.TimestampFileMetaEqual(meta, m) - return err == nil -} - type Destination interface { io.Writer Delete() error @@ -829,11 +787,11 @@ type Destination interface { // // dest will be deleted and an error returned in the following situations: // -// * The target does not exist in the local targets.json -// * Failed to fetch the chain of delegations accessible from local snapshot.json -// * The target does not exist in any targets -// * Metadata cannot be generated for the downloaded data -// * Generated metadata does not match local metadata for the given file +// - The target does not exist in the local targets.json +// - Failed to fetch the chain of delegations accessible from local snapshot.json +// - The target does not exist in any targets +// - Metadata cannot be generated for the downloaded data +// - Generated metadata does not match local metadata for the given file func (c *Client) Download(name string, dest Destination) (err error) { // delete dest if there is an error defer func() { diff --git a/client/client_test.go b/client/client_test.go index f312f847f..db8c3272c 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2,11 +2,12 @@ package client import ( "bytes" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" "io" - "io/ioutil" "net" "net/http" "os" @@ -398,7 +399,7 @@ func newClientWithMeta(baseDir string, relPath string, serverAddr string) (*Clie c := NewClient(MemoryLocalStore(), remote) for _, m := range []string{"root.json", "snapshot.json", "timestamp.json", "targets.json"} { if _, err := os.Stat(initialStateDir + "/" + m); err == nil { - metadataJSON, err := ioutil.ReadFile(initialStateDir + "/" + m) + metadataJSON, err := os.ReadFile(initialStateDir + "/" + m) if err != nil { return nil, err } @@ -1211,7 +1212,7 @@ func generateRepoFS(c *C, dir string, files map[string][]byte, consistentSnapsho for file, data := range files { path := filepath.Join(dir, "staged", "targets", file) c.Assert(os.MkdirAll(filepath.Dir(path), 0755), IsNil) - c.Assert(ioutil.WriteFile(path, data, 0644), IsNil) + c.Assert(os.WriteFile(path, data, 0644), IsNil) c.Assert(repo.AddTarget(file, nil), IsNil) } c.Assert(repo.Snapshot(), IsNil) @@ -1237,3 +1238,136 @@ func (s *ClientSuite) TestVerifyDigest(c *C) { c.Assert(client.VerifyDigest(hash, "sha256", size, digest), IsNil) } + +type StateLessSuite struct{} + +var _ = Suite(&StateLessSuite{}) + +func (s *StateLessSuite) TestRejectsMultiSignaturesSameKeyDifferentIDs(c *C) { + // In this test Alice and Bob want to create a TUF repo + // where a root key rotation would require both their signatures. + // Alice uses an old version of Go-TUF where each key gets assigned several IDs. + // Bob uses a modern version of Go-TUF that does not produce the same list of IDs for a same key. + // This test checks that the TUF client + // will not accept a root rotation + // signed twice with Alice's key with different key IDs each time. + // This test was failing with https://github.com/theupdateframework/go-tuf/tree/ac7b5d7bce18cca5a84a28b021bd6372f450b35b + // because the signature verification code was assuming that the key IDs used in the metadata + // were the same as the one the TUF library of the client would generate, + // breaking the security of threshold signatures. + + // The attack works just the same if Alice is malicious from the beginning + // and convinces Bob to sign an initial "root.json" + // with additional key IDs for her only key, + // but this scenario show that the vulnerability can even impact situations + // where Alice is not malicious at all, + // she was simply using an old client and an attacker stole her key. + // The purpose of threshold signatures in TUF is precisely + // to make sure that an attacker cannot forge signatures + // if they did not steal a large enough number of keys. + + alice, err := keys.GenerateEd25519Key() + if err != nil { + panic(err) + } + + root := data.NewRoot() + root.Version = 1 + root.Roles["root"] = &data.Role{ + KeyIDs: []string{}, + Threshold: 2, // Note the threshold + } + + // reproduces how IDs were computed in + // https://github.com/theupdateframework/go-tuf/blob/8e84384bebe3/data/types.go#L50 + oldTUFIDs := func(k *data.PublicKey) []string { + bytes, _ := cjson.EncodeCanonical(k) + digest := sha256.Sum256(bytes) + ids := []string{hex.EncodeToString(digest[:])} + + if k.Scheme != "" || len(k.Algorithms) != 0 { + bytes, _ = cjson.EncodeCanonical(&data.PublicKey{ + Type: k.Type, + Value: k.Value, + }) + digest = sha256.Sum256(bytes) + ids = append(ids, hex.EncodeToString(digest[:])) + } + + return ids + } + + // Alice adds her key using an old version of go-tuf + // which will use several IDs + for _, keyID := range oldTUFIDs(alice.PublicData()) { + root.Keys[keyID] = alice.PublicData() + root.Roles["root"].KeyIDs = append(root.Roles["root"].KeyIDs, keyID) + } + + bob, err := keys.GenerateEd25519Key() + if err != nil { + panic(err) + } + + root.AddKey(bob.PublicData()) + root.Roles["root"].KeyIDs = append( + root.Roles["root"].KeyIDs, + bob.PublicData().IDs()..., + ) + + // signer for the other roles, not important + delegatedSigner, _ := keys.GenerateEd25519Key() + root.AddKey(delegatedSigner.PublicData()) + for _, role := range []string{"targets", "snapshot", "timestamp"} { + root.Roles[role] = &data.Role{ + KeyIDs: delegatedSigner.PublicData().IDs(), + Threshold: 1, + } + } + + signedRoot, err := sign.Marshal(root, alice, bob) + c.Assert(err, IsNil) + rootJSON, err := json.Marshal(signedRoot) + c.Assert(err, IsNil) + + // producing evil root using only Alice's key + + evilRoot := root + evilRoot.Version = 2 + + canonical, err := cjson.EncodeCanonical(evilRoot) + c.Assert(err, IsNil) + sig, err := alice.SignMessage(canonical) + c.Assert(err, IsNil) + signedEvilRoot := &data.Signed{ + Signed: canonical, + Signatures: make([]data.Signature, 0), + } + for _, keyID := range oldTUFIDs(alice.PublicData()) { + signedEvilRoot.Signatures = append(signedEvilRoot.Signatures, data.Signature{ + Signature: sig, + KeyID: keyID, + }) + } + evilRootJSON, err := json.Marshal(signedEvilRoot) + c.Assert(err, IsNil) + + // checking that client does not accept root rotation + // to evil root + + localStore := MemoryLocalStore() + err = localStore.SetMeta("root.json", rootJSON) + c.Assert(err, IsNil) + + remoteStore := newFakeRemoteStore() + remoteStore.meta["2.root.json"] = newFakeFile(evilRootJSON) + + client := NewClient(localStore, remoteStore) + + err = client.UpdateRoots() + if err != nil { + c.Assert(err, DeepEquals, verify.ErrRoleThreshold{Expected: 2, Actual: 1}) + } else { + c.Fatalf("client returned no error when updating with evil root") + } +} diff --git a/client/delegations_test.go b/client/delegations_test.go index 250e7b700..47fdc229b 100644 --- a/client/delegations_test.go +++ b/client/delegations_test.go @@ -5,9 +5,9 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "net" "net/http" + "os" "strings" "testing" "time" @@ -273,10 +273,10 @@ func initTestDelegationClient(t *testing.T, dirPrefix string) (*Client, func() e assert.Nil(t, err) c := NewClient(MemoryLocalStore(), remote) - rawFile, err := ioutil.ReadFile(initialStateDir + "/" + "root.json") + rawFile, err := os.ReadFile(initialStateDir + "/" + "root.json") assert.Nil(t, err) assert.Nil(t, c.Init(rawFile)) - files, err := ioutil.ReadDir(initialStateDir) + files, err := os.ReadDir(initialStateDir) assert.Nil(t, err) // load local files @@ -287,7 +287,7 @@ func initTestDelegationClient(t *testing.T, dirPrefix string) (*Client, func() e name := f.Name() // ignoring consistent snapshot when loading initial state if len(strings.Split(name, ".")) == 1 && strings.HasSuffix(name, ".json") { - rawFile, err := ioutil.ReadFile(initialStateDir + "/" + name) + rawFile, err := os.ReadFile(initialStateDir + "/" + name) assert.Nil(t, err) assert.Nil(t, c.local.SetMeta(name, rawFile)) } diff --git a/client/interop_test.go b/client/interop_test.go index 15ece0d33..1fb4f9827 100644 --- a/client/interop_test.go +++ b/client/interop_test.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "net" "net/http" "os" @@ -33,7 +32,7 @@ func checkGoIdentity(c *C, consistentSnapshot bool) { c.Assert(err, IsNil) testDataDir := filepath.Join(cwd, "testdata") - tempDir, err := ioutil.TempDir("", "") + tempDir, err := os.MkdirTemp("", "") c.Assert(err, IsNil) defer os.RemoveAll(tempDir) @@ -59,7 +58,7 @@ func computeHashes(c *C, dir string) map[string]string { return nil } - bytes, err := ioutil.ReadFile(path) + bytes, err := os.ReadFile(path) if err != nil { return err } @@ -108,7 +107,7 @@ func newTestCase(c *C, name string, consistentSnapshot bool, options *HTTPRemote c.Assert(err, IsNil) testDir := filepath.Join(cwd, "testdata", name, fmt.Sprintf("consistent-snapshot-%t", consistentSnapshot)) - dirEntries, err := ioutil.ReadDir(testDir) + dirEntries, err := os.ReadDir(testDir) c.Assert(err, IsNil) c.Assert(dirEntries, Not(HasLen), 0) diff --git a/client/python_interop/python_interop_test.go b/client/python_interop/python_interop_test.go index 9e7024f1c..cd968f93c 100644 --- a/client/python_interop/python_interop_test.go +++ b/client/python_interop/python_interop_test.go @@ -3,7 +3,6 @@ package client import ( "bytes" "fmt" - "io/ioutil" "net" "net/http" "net/url" @@ -58,7 +57,7 @@ func (InteropSuite) TestGoClientPythonGenerated(c *C) { // initiate a client with the root metadata client := client.NewClient(client.MemoryLocalStore(), remote) - rootJSON, err := ioutil.ReadFile(filepath.Join(testDataDir, dir, "repository", "metadata", "1.root.json")) + rootJSON, err := os.ReadFile(filepath.Join(testDataDir, dir, "repository", "metadata", "1.root.json")) c.Assert(err, IsNil) c.Assert(client.Init(rootJSON), IsNil) @@ -99,7 +98,7 @@ func generateRepoFS(c *C, dir string, files map[string][]byte, consistentSnapsho for file, data := range files { path := filepath.Join(dir, "staged", "targets", file) c.Assert(os.MkdirAll(filepath.Dir(path), 0755), IsNil) - c.Assert(ioutil.WriteFile(path, data, 0644), IsNil) + c.Assert(os.WriteFile(path, data, 0644), IsNil) c.Assert(repo.AddTarget(file, nil), IsNil) } c.Assert(repo.Snapshot(), IsNil) @@ -134,9 +133,9 @@ func (InteropSuite) TestPythonClientGoGenerated(c *C) { prevDir := filepath.Join(clientDir, "tufrepo", "metadata", "previous") c.Assert(os.MkdirAll(currDir, 0755), IsNil) c.Assert(os.MkdirAll(prevDir, 0755), IsNil) - rootJSON, err := ioutil.ReadFile(filepath.Join(dir, "repository", "1.root.json")) + rootJSON, err := os.ReadFile(filepath.Join(dir, "repository", "1.root.json")) c.Assert(err, IsNil) - c.Assert(ioutil.WriteFile(filepath.Join(currDir, "root.json"), rootJSON, 0644), IsNil) + c.Assert(os.WriteFile(filepath.Join(currDir, "root.json"), rootJSON, 0644), IsNil) args := []string{ filepath.Join(cwd, "testdata", "python-tuf-v1.0.0", "client.py"), @@ -155,7 +154,7 @@ func (InteropSuite) TestPythonClientGoGenerated(c *C) { // check the target files got downloaded for path, expected := range files { - actual, err := ioutil.ReadFile(filepath.Join(clientDir, "tuftargets", url.QueryEscape(path))) + actual, err := os.ReadFile(filepath.Join(clientDir, "tuftargets", url.QueryEscape(path))) c.Assert(err, IsNil) c.Assert(actual, DeepEquals, expected) } diff --git a/client/testdata/go-tuf-transition-M3/generate.go b/client/testdata/go-tuf-transition-M3/generate.go index 20cb8dc16..2a9d20f35 100644 --- a/client/testdata/go-tuf-transition-M3/generate.go +++ b/client/testdata/go-tuf-transition-M3/generate.go @@ -3,7 +3,6 @@ package main import ( "encoding/json" "fmt" - "io/ioutil" "log" "os" "os/exec" @@ -64,7 +63,7 @@ func addTargets(repo *tuf.Repo, dir string, files map[string][]byte) { for file, data := range files { path := filepath.Join(dir, "staged", "targets", file) assertNoError(os.MkdirAll(filepath.Dir(path), 0755)) - assertNoError(ioutil.WriteFile(path, data, 0644)) + assertNoError(os.WriteFile(path, data, 0644)) paths = append(paths, file) } assertNoError(repo.AddTargetsWithExpires(paths, nil, expirationDate)) diff --git a/client/testdata/go-tuf-transition-M4/generate.go b/client/testdata/go-tuf-transition-M4/generate.go index 552474c6f..6dc567c41 100644 --- a/client/testdata/go-tuf-transition-M4/generate.go +++ b/client/testdata/go-tuf-transition-M4/generate.go @@ -3,7 +3,6 @@ package main import ( "encoding/json" "fmt" - "io/ioutil" "log" "os" "os/exec" @@ -62,7 +61,7 @@ func addTargets(repo *tuf.Repo, dir string, files map[string][]byte) { for file, data := range files { path := filepath.Join(dir, "staged", "targets", file) assertNoError(os.MkdirAll(filepath.Dir(path), 0755)) - assertNoError(ioutil.WriteFile(path, data, 0644)) + assertNoError(os.WriteFile(path, data, 0644)) paths = append(paths, file) } assertNoError(repo.AddTargetsWithExpires(paths, nil, expirationDate)) diff --git a/client/testdata/go-tuf/generator/generator.go b/client/testdata/go-tuf/generator/generator.go index eb6aaa424..43903ace2 100644 --- a/client/testdata/go-tuf/generator/generator.go +++ b/client/testdata/go-tuf/generator/generator.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "io/fs" - "io/ioutil" "log" "os" "path/filepath" @@ -97,7 +96,7 @@ func addTargets(repo *tuf.Repo, dir string, files map[string][]byte) { for file, data := range files { path := filepath.Join(dir, "staged", "targets", file) assertNoError(os.MkdirAll(filepath.Dir(path), 0755)) - assertNoError(ioutil.WriteFile(path, data, 0644)) + assertNoError(os.WriteFile(path, data, 0644)) paths = append(paths, file) } assertNoError(repo.AddTargetsWithExpires(paths, nil, expirationDate)) diff --git a/client/testdata/tools/gen-keys.go b/client/testdata/tools/gen-keys.go index f592d1508..f4db562ef 100644 --- a/client/testdata/tools/gen-keys.go +++ b/client/testdata/tools/gen-keys.go @@ -7,7 +7,7 @@ package main import ( "encoding/json" "fmt" - "io/ioutil" + "os" "time" "github.com/theupdateframework/go-tuf/data" @@ -40,7 +40,7 @@ func main() { s, err := json.MarshalIndent(&roles, "", " ") assertNoError(err) - ioutil.WriteFile("keys.json", []byte(s), 0644) + os.WriteFile("keys.json", []byte(s), 0644) } func assertNoError(err error) { diff --git a/client/testdata/tools/linkify-metadata.go b/client/testdata/tools/linkify-metadata.go index 0ccd3a4a9..178868087 100644 --- a/client/testdata/tools/linkify-metadata.go +++ b/client/testdata/tools/linkify-metadata.go @@ -7,7 +7,6 @@ package main import ( "crypto/sha256" "fmt" - "io/ioutil" "log" "os" "path/filepath" @@ -55,7 +54,7 @@ func linkifyDir(rootDir string) error { } func readStepDirs(rootDir string) ([]string, error) { - dirEntries, err := ioutil.ReadDir(rootDir) + dirEntries, err := os.ReadDir(rootDir) if err != nil { return []string{}, err } @@ -79,7 +78,7 @@ func computeHashes(dir string) map[string][32]byte { return nil } - bytes, err := ioutil.ReadFile(path) + bytes, err := os.ReadFile(path) if err != nil { return err } diff --git a/cmd/tuf-client/get.go b/cmd/tuf-client/get.go index 1f60429fb..60df6461b 100644 --- a/cmd/tuf-client/get.go +++ b/cmd/tuf-client/get.go @@ -2,7 +2,6 @@ package main import ( "io" - "io/ioutil" "os" "github.com/flynn/go-docopt" @@ -35,7 +34,7 @@ func cmdGet(args *docopt.Args, client *tuf.Client) error { return err } target := util.NormalizeTarget(args.String[""]) - file, err := ioutil.TempFile("", "go-tuf") + file, err := os.CreateTemp("", "go-tuf") if err != nil { return err } diff --git a/local_store.go b/local_store.go index 9fd95dc53..34038f350 100644 --- a/local_store.go +++ b/local_store.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "io/fs" - "io/ioutil" "os" "path/filepath" "strings" @@ -276,7 +275,7 @@ func (f *fileSystemStore) GetMeta() (map[string]json.RawMessage, error) { meta := make(map[string]json.RawMessage) for name, path := range metaPaths { - f, err := ioutil.ReadFile(path) + f, err := os.ReadFile(path) if err != nil { return nil, err } diff --git a/repo_test.go b/repo_test.go index 94943feb8..2f3aebb4d 100644 --- a/repo_test.go +++ b/repo_test.go @@ -9,7 +9,6 @@ import ( "encoding/json" "errors" "fmt" - "io/ioutil" "os" "path" "path/filepath" @@ -908,7 +907,8 @@ func (t *tmpDir) assertEmpty(dir string) { } t.c.Assert(err, IsNil) t.c.Assert(f.IsDir(), Equals, true) - entries, err := ioutil.ReadDir(path) + + entries, err := os.ReadDir(path) t.c.Assert(err, IsNil) // check that all (if any) entries are also empty for _, e := range entries { @@ -928,12 +928,12 @@ func (t *tmpDir) stagedTargetPath(path string) string { func (t *tmpDir) writeStagedTarget(path, data string) { path = t.stagedTargetPath(path) t.c.Assert(os.MkdirAll(filepath.Dir(path), 0755), IsNil) - t.c.Assert(ioutil.WriteFile(path, []byte(data), 0644), IsNil) + t.c.Assert(os.WriteFile(path, []byte(data), 0644), IsNil) } func (t *tmpDir) readFile(path string) []byte { t.assertExists(path) - data, err := ioutil.ReadFile(filepath.Join(t.path, path)) + data, err := os.ReadFile(filepath.Join(t.path, path)) t.c.Assert(err, IsNil) return data } @@ -2685,7 +2685,7 @@ func (rs *RepoSuite) TestTargetMetadataLength(c *C) { } s := &data.Signed{} c.Assert(json.Unmarshal(targetsJSON, s), IsNil) - fmt.Fprintf(os.Stderr, string(s.Signed)) + fmt.Fprint(os.Stderr, s.Signed) var objMap map[string]json.RawMessage c.Assert(json.Unmarshal(s.Signed, &objMap), IsNil) targetsMap, ok := objMap["targets"] @@ -2698,10 +2698,13 @@ func (rs *RepoSuite) TestTargetMetadataLength(c *C) { c.Fatal("missing foo.txt in targets") } c.Assert(json.Unmarshal(targetsMap, &objMap), IsNil) - targetsMap, ok = objMap["length"] + lengthMsg, ok := objMap["length"] if !ok { c.Fatal("missing length field in foo.txt file meta") } + var length int64 + c.Assert(json.Unmarshal(lengthMsg, &length), IsNil) + c.Assert(length, Equals, int64(0)) } func (rs *RepoSuite) TestDeprecatedHexEncodedKeysFails(c *C) { diff --git a/util/util.go b/util/util.go index 0878c8235..049169db1 100644 --- a/util/util.go +++ b/util/util.go @@ -10,7 +10,6 @@ import ( "fmt" "hash" "io" - "io/ioutil" "os" "path" "path/filepath" @@ -202,7 +201,7 @@ func GenerateFileMeta(r io.Reader, hashAlgorithms ...string) (data.FileMeta, err hashes[hashAlgorithm] = h r = io.TeeReader(r, h) } - n, err := io.Copy(ioutil.Discard, r) + n, err := io.Copy(io.Discard, r) if err != nil { return data.FileMeta{}, err } @@ -218,7 +217,7 @@ type versionedMeta struct { } func generateVersionedFileMeta(r io.Reader, hashAlgorithms ...string) (data.FileMeta, int64, error) { - b, err := ioutil.ReadAll(r) + b, err := io.ReadAll(r) if err != nil { return data.FileMeta{}, 0, err } @@ -301,7 +300,7 @@ func HashedPaths(p string, hashes data.Hashes) []string { func AtomicallyWriteFile(filename string, data []byte, perm os.FileMode) error { dir, name := filepath.Split(filename) - f, err := ioutil.TempFile(dir, name) + f, err := os.CreateTemp(dir, name) if err != nil { return err } diff --git a/verify/db_test.go b/verify/db_test.go index 2e26b3ceb..01d7a3081 100644 --- a/verify/db_test.go +++ b/verify/db_test.go @@ -93,7 +93,6 @@ func TestDelegationsDB(t *testing.T) { // Previously, every key's key ID was the SHA256 of the public key. TAP-12 // allows arbitrary key IDs, with no loss in security. // -// // TAP-12: https://github.com/theupdateframework/taps/blob/master/tap12.md func TestTAP12(t *testing.T) { db := NewDB() diff --git a/verify/verify.go b/verify/verify.go index c12aaaf85..f5675a250 100644 --- a/verify/verify.go +++ b/verify/verify.go @@ -92,8 +92,8 @@ func (db *DB) VerifySignatures(s *data.Signed, role string) error { // Verify that a threshold of keys signed the data. Since keys can have // multiple key ids, we need to protect against multiple attached // signatures that just differ on the key id. - seen := make(map[string]struct{}) - valid := 0 + verifiedKeyIDs := make(map[string]struct{}) + numVerifiedKeys := 0 for _, sig := range s.Signatures { if !roleData.ValidKey(sig.KeyID) { continue @@ -104,21 +104,32 @@ func (db *DB) VerifySignatures(s *data.Signed, role string) error { } if err := verifier.Verify(msg, sig.Signature); err != nil { + // FIXME: don't err out on the 1st bad signature. return ErrInvalid } // Only consider this key valid if we haven't seen any of it's // key ids before. - if _, ok := seen[sig.KeyID]; !ok { - for _, id := range verifier.MarshalPublicKey().IDs() { - seen[id] = struct{}{} + // Careful: we must not rely on the key IDs _declared in the file_, + // instead we get to decide what key IDs this key correspond to. + // XXX dangerous; better stop supporting multiple key IDs altogether. + keyIDs := verifier.MarshalPublicKey().IDs() + wasKeySeen := false + for _, keyID := range keyIDs { + if _, present := verifiedKeyIDs[keyID]; present { + wasKeySeen = true + } + } + if !wasKeySeen { + for _, id := range keyIDs { + verifiedKeyIDs[id] = struct{}{} } - valid++ + numVerifiedKeys++ } } - if valid < roleData.Threshold { - return ErrRoleThreshold{roleData.Threshold, valid} + if numVerifiedKeys < roleData.Threshold { + return ErrRoleThreshold{roleData.Threshold, numVerifiedKeys} } return nil