From a4aa40ceadefda1bc387d93799cf18439c86ea87 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 21 Jun 2022 20:01:04 +0300 Subject: [PATCH 1/5] feat: Verify decompressed coder binaries via sha256 --- scripts/build_go_slim.sh | 16 +++- site/site.go | 77 +++++++++++++++++- site/site_test.go | 172 +++++++++++++++++++++++++++++++-------- 3 files changed, 224 insertions(+), 41 deletions(-) diff --git a/scripts/build_go_slim.sh b/scripts/build_go_slim.sh index c5b029ec44c1c..e4328f6988fdd 100755 --- a/scripts/build_go_slim.sh +++ b/scripts/build_go_slim.sh @@ -58,7 +58,7 @@ done # Check dependencies dependencies go if [[ $compress != 0 ]]; then - dependencies tar zstd + dependencies shasum tar zstd if [[ $compress != [0-9]* ]] || [[ $compress -gt 22 ]] || [[ $compress -lt 1 ]]; then error "Invalid value for compress, must in in the range of [1, 22]" @@ -110,10 +110,20 @@ for f in ./coder-slim_*; do done if [[ $compress != 0 ]]; then - log "--- Compressing coder-slim binaries using zstd level $compress ($dest_dir/coder.tar.zst)" pushd "$dest_dir" - tar cf coder.tar coder-* + log "--- Generating SHA256 for coder-slim binaries ($dest_dir/coder.sha256)" + shasum -b -a 256 coder-* | tee coder.sha256 + echo "$dest_dir/coder.sha256" + log + log + + log "--- Compressing coder-slim binaries using zstd level $compress ($dest_dir/coder.tar.zst)" + tar cf coder.tar coder.sha256 coder-* rm coder-* zstd --force --ultra --long -"${compress}" --rm --no-progress coder.tar -o coder.tar.zst + echo "$dest_dir/coder.tar.zst" + log + log + popd fi diff --git a/site/site.go b/site/site.go index 65d21591618fb..e7877d99aaccc 100644 --- a/site/site.go +++ b/site/site.go @@ -4,8 +4,11 @@ import ( "archive/tar" "bytes" "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" + "hash" "io" "io/fs" "net/http" @@ -439,12 +442,18 @@ func ExtractOrReadBinFS(dest string, siteFS fs.FS) (http.FileSystem, error) { return nil, err } - n, err := extractBin(dest, archive) + ok, err := verifyBinSha265IsCurrent(dest, siteFS) if err != nil { - return nil, xerrors.Errorf("extract coder binaries failed: %w", err) + return nil, xerrors.Errorf("verify coder binaries sha256 failed: %w", err) } - if n == 0 { - return nil, xerrors.New("no files were extracted from coder binaries archive") + if !ok { + n, err := extractBin(dest, archive) + if err != nil { + return nil, xerrors.Errorf("extract coder binaries failed: %w", err) + } + if n == 0 { + return nil, xerrors.New("no files were extracted from coder binaries archive") + } } return dir, nil @@ -461,6 +470,66 @@ func filterFiles(files []fs.DirEntry, names ...string) []fs.DirEntry { return filtered } +func verifyBinSha265IsCurrent(dest string, siteFS fs.FS) (ok bool, err error) { + b1, err := fs.ReadFile(siteFS, "bin/coder.sha256") + if err != nil { + return false, xerrors.Errorf("read coder sha256 from embedded fs failed: %w", err) + } + // Parse sha256 file. + shaFiles := make(map[string][]byte) + for _, line := range bytes.Split(bytes.TrimSpace(b1), []byte{'\n'}) { + parts := bytes.Split(line, []byte{' ', '*'}) + if len(parts) != 2 { + return false, xerrors.Errorf("malformed sha256 file: %w", err) + } + shaFiles[string(parts[1])] = parts[0] + } + if len(shaFiles) == 0 { + return false, xerrors.Errorf("empty sha256 file: %w", err) + } + + b2, err := os.ReadFile(filepath.Join(dest, "coder.sha256")) + if err != nil { + if xerrors.Is(err, fs.ErrNotExist) { + return false, nil + } + return false, xerrors.Errorf("read coder sha256 failed: %w", err) + } + + // Verify the hash of each on-disk binary. + sha := sha256.New() + for file, hash1 := range shaFiles { + sha.Reset() + hash2, err := hashFile(sha, filepath.Join(dest, file)) + if err != nil { + return false, xerrors.Errorf("hash file failed: %w", err) + } + if !bytes.Equal(hash1, hash2) { + return false, nil + } + } + + return bytes.Equal(b1, b2), nil +} + +func hashFile(sum hash.Hash, name string) ([]byte, error) { + f, err := os.Open(name) + if err != nil { + return nil, err + } + defer f.Close() + + _, err = io.Copy(sum, f) + if err != nil { + return nil, err + } + + b := make([]byte, sum.Size()) + sum.Sum(b[:0]) + + return []byte(hex.EncodeToString(b)), nil +} + func extractBin(dest string, r io.Reader) (numExtraced int, err error) { opts := []zstd.DOption{ // Concurrency doesn't help us when decoding the tar and diff --git a/site/site_test.go b/site/site_test.go index 648d279bc2552..83a28bb059fb0 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -1,12 +1,17 @@ package site_test import ( + "bufio" + "bytes" "context" "encoding/json" "fmt" "io" + "io/fs" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" "testing/fstest" @@ -175,6 +180,43 @@ func TestShouldCacheFile(t *testing.T) { } } +const ( + binCoderSha256 = "bin/coder.sha256" + binCoderTarZstd = "bin/coder.tar.zst" +) + +func sampleBinFS() fstest.MapFS { + return fstest.MapFS{ + binCoderSha256: &fstest.MapFile{ + Data: []byte("9da308c2e4bc33afa72df5c088b5fc5673c477f3ef21d6bdaa358393834f9804 *coder-linux-amd64\n"), + }, + binCoderTarZstd: &fstest.MapFile{ + // echo -n compressed >coder-linux-amd64 + // shasum -b -a 256 coder-linux-amd64 | tee coder.sha256 + // tar cf coder.tar coder-linux-amd64 + // zstd --long --ultra -22 coder.tar + Data: []byte{ + 0x28, 0xb5, 0x2f, 0xfd, 0x64, 0x00, 0x27, 0x4d, 0x05, 0x00, 0xf2, 0x49, + 0x1e, 0x1a, 0x90, 0xb7, 0x0d, 0x00, 0x8d, 0x0d, 0x68, 0x68, 0xea, 0x6b, + 0x77, 0x17, 0xec, 0xce, 0xb2, 0x97, 0x24, 0x25, 0x45, 0x2e, 0x4d, 0xbf, + 0xeb, 0x46, 0x94, 0x01, 0xa7, 0x56, 0x6b, 0x4b, 0x39, 0x20, 0x49, 0x3a, + 0xab, 0xda, 0xae, 0x89, 0x07, 0x60, 0x57, 0xc8, 0x17, 0x89, 0xc5, 0x40, + 0x28, 0x14, 0x01, 0xc3, 0x88, 0x0c, 0xc6, 0x8b, 0xd6, 0x58, 0xd4, 0xfb, + 0xaf, 0x0e, 0x57, 0xa9, 0x6c, 0x2d, 0x3b, 0x87, 0xd3, 0x1e, 0xef, 0xc4, + 0xfa, 0x40, 0x08, 0x53, 0x33, 0xa5, 0xe7, 0xd9, 0xef, 0xa6, 0x83, 0x97, + 0x26, 0x84, 0xfb, 0xca, 0x38, 0xf6, 0xc8, 0x19, 0xa3, 0x3c, 0xa2, 0x2d, + 0x87, 0xe6, 0x1c, 0x35, 0x13, 0xcf, 0x8f, 0x2b, 0x2f, 0xb7, 0x72, 0x1e, + 0x6b, 0x62, 0x9f, 0x64, 0x76, 0x3c, 0x6f, 0xfd, 0x41, 0x13, 0x28, 0x4b, + 0x25, 0x29, 0x11, 0x20, 0x50, 0x2b, 0x2e, 0x6d, 0x03, 0xf2, 0x21, 0xef, + 0xc7, 0xa8, 0x04, 0xc0, 0xb5, 0x0d, 0x1c, 0x20, 0x53, 0x08, 0xee, 0x0c, + 0xa0, 0x62, 0xa5, 0x42, 0x11, 0xc1, 0xa0, 0x49, 0x36, 0x5a, 0x21, 0x93, + 0x04, 0x69, 0x38, 0x3b, 0x20, 0xb3, 0x03, 0x5c, 0x34, 0xf5, 0x2f, 0x1f, + 0xeb, 0x39, 0xe8, + }, + }, + } +} + func TestServingBin(t *testing.T) { t.Parallel() @@ -194,6 +236,12 @@ func TestServingBin(t *testing.T) { }, } + sampleBinFSCorrupted := sampleBinFS() + copy(sampleBinFSCorrupted[binCoderTarZstd].Data[10:], bytes.Repeat([]byte{0}, 10)) // Zero portion of archive. + + sampleBinFSMissingSha256 := sampleBinFS() + delete(sampleBinFSMissingSha256, binCoderSha256) + type req struct { url string wantStatus int @@ -207,52 +255,35 @@ func TestServingBin(t *testing.T) { }{ { name: "Extract and serve bin", - fs: fstest.MapFS{ - "bin/coder.tar.zst": &fstest.MapFile{ - // echo iamcoder >coder-linux-amd64 - // tar cf coder.tar coder-linux-amd64 - // zstd --long --ultra -22 coder.tar - Data: []byte{ - 0x28, 0xb5, 0x2f, 0xfd, 0x64, 0x00, 0x27, 0xf5, 0x02, 0x00, 0x12, 0xc4, - 0x0e, 0x16, 0xa0, 0xb5, 0x39, 0x00, 0xe8, 0x67, 0x59, 0xaf, 0xe3, 0xdd, - 0x8d, 0xfe, 0x47, 0xe8, 0x9d, 0x9c, 0x44, 0x0b, 0x75, 0x70, 0x61, 0x52, - 0x0d, 0x56, 0xaa, 0x16, 0xb9, 0x5a, 0x0a, 0x4b, 0x40, 0xd2, 0x7a, 0x05, - 0xd1, 0xd7, 0xe3, 0xf9, 0xf9, 0x07, 0xef, 0xda, 0x77, 0x04, 0xff, 0xe8, - 0x7a, 0x94, 0x56, 0x9a, 0x40, 0x3b, 0x94, 0x61, 0x18, 0x91, 0x90, 0x21, - 0x0c, 0x00, 0xf3, 0xc5, 0xe5, 0xd8, 0x80, 0x10, 0x06, 0x0a, 0x08, 0x86, - 0xb2, 0x00, 0x60, 0x12, 0x70, 0xd3, 0x51, 0x05, 0x04, 0x20, 0x16, 0x2c, - 0x79, 0xad, 0x01, 0xc0, 0xf5, 0x28, 0x08, 0x03, 0x1c, 0x4c, 0x84, 0xf4, - }, - }, - }, + fs: sampleBinFS(), reqs: []req{ - {url: "/bin/coder-linux-amd64", wantStatus: http.StatusOK, wantBody: []byte("iamcoder\n")}, + {url: "/bin/coder-linux-amd64", wantStatus: http.StatusOK, wantBody: []byte("compressed")}, {url: "/bin/GITKEEP", wantStatus: http.StatusNotFound}, }, }, { - name: "Error on invalid archive", + name: "Extract and serve bin fails due to missing sha256", + fs: sampleBinFSMissingSha256, + wantErr: true, + }, + { + name: "Error on invalid archive", + fs: sampleBinFSCorrupted, + wantErr: true, + }, + { + name: "Error on malformed sha256 file", fs: fstest.MapFS{ - "bin/coder.tar.zst": &fstest.MapFile{ - Data: []byte{ - 0x28, 0xb5, 0x2f, 0xfd, 0x64, 0x00, 0x27, 0xf5, 0x02, 0x00, 0x12, 0xc4, - 0x0e, 0x16, 0xa0, 0xb5, 0x39, 0x00, 0xe8, 0x67, 0x59, 0xaf, 0xe3, 0xdd, - 0x8d, 0xfe, 0x47, 0xe8, 0x9d, 0x9c, 0x44, 0x0b, 0x75, 0x70, 0x61, 0x52, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Zeroed from above test. - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Zeroed from above test. - 0x7a, 0x94, 0x56, 0x9a, 0x40, 0x3b, 0x94, 0x61, 0x18, 0x91, 0x90, 0x21, - 0x0c, 0x00, 0xf3, 0xc5, 0xe5, 0xd8, 0x80, 0x10, 0x06, 0x0a, 0x08, 0x86, - 0xb2, 0x00, 0x60, 0x12, 0x70, 0xd3, 0x51, 0x05, 0x04, 0x20, 0x16, 0x2c, - 0x79, 0xad, 0x01, 0xc0, 0xf5, 0x28, 0x08, 0x03, 0x1c, 0x4c, 0x84, 0xf4, - }, - }, + binCoderSha256: &fstest.MapFile{Data: []byte("byebye")}, + binCoderTarZstd: sampleBinFS()[binCoderTarZstd], }, wantErr: true, }, { name: "Error on empty archive", fs: fstest.MapFS{ - "bin/coder.tar.zst": &fstest.MapFile{Data: []byte{}}, + binCoderSha256: &fstest.MapFile{Data: []byte{}}, + binCoderTarZstd: &fstest.MapFile{Data: []byte{}}, }, wantErr: true, }, @@ -336,6 +367,79 @@ func TestServingBin(t *testing.T) { } } +func TestExtractOrReadBinFS(t *testing.T) { + t.Parallel() + t.Run("DoubleExtractDoesNotModifyFiles", func(t *testing.T) { + t.Parallel() + + siteFS := sampleBinFS() + dest := t.TempDir() + _, err := site.ExtractOrReadBinFS(dest, siteFS) + require.NoError(t, err) + + checkModtime := func() map[string]time.Time { + m := make(map[string]time.Time) + + err = filepath.WalkDir(dest, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + stat, err := d.Info() + if err != nil { + return err + } + + m[path] = stat.ModTime() + return nil + }) + require.NoError(t, err) + + return m + } + + firstModtimes := checkModtime() + + _, err = site.ExtractOrReadBinFS(dest, siteFS) + require.NoError(t, err) + + secondModtimes := checkModtime() + + assert.Equal(t, firstModtimes, secondModtimes, "second extract should not modify files") + }) + t.Run("SHA256MismatchCausesReExtract", func(t *testing.T) { + t.Parallel() + + siteFS := sampleBinFS() + dest := t.TempDir() + _, err := site.ExtractOrReadBinFS(dest, siteFS) + require.NoError(t, err) + + bin := filepath.Join(dest, "bin", "coder-linux-amd64") + f, err := os.OpenFile(bin, os.O_WRONLY, 0o600) + require.NoError(t, err) + + dontWant := []byte("hello") + _, err = f.WriteAt(dontWant, 0) // Overwrite the start of file. + assert.NoError(t, err) // Assert to allow f.Close. + + err = f.Close() + require.NoError(t, err) + + _, err = site.ExtractOrReadBinFS(dest, siteFS) + require.NoError(t, err) + + f, err = os.Open(bin) + require.NoError(t, err) + defer f.Close() + + got := make([]byte, 5) // hello + _, err = bufio.NewReader(f).Read(got) + require.NoError(t, err) + + assert.NotEqual(t, dontWant, got, "file should be overwritten on hash mismatch") + }) +} + func TestServeAPIResponse(t *testing.T) { t.Parallel() From 7345409871113ee4983f9d41b9972f48177df6ba Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 21 Jun 2022 20:15:46 +0300 Subject: [PATCH 2/5] fix: Ignore mtime on dirs in test --- site/site_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/site/site_test.go b/site/site_test.go index 83a28bb059fb0..510a211cd3e08 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -384,6 +384,9 @@ func TestExtractOrReadBinFS(t *testing.T) { if err != nil { return err } + if d.IsDir() { + return nil // Only check the files. + } stat, err := d.Info() if err != nil { return err From 5609369e521c1e2354e57d7f282e69f6857cd244 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 22 Jun 2022 12:36:22 +0300 Subject: [PATCH 3/5] fix: Hash concurrently and use sha1 vs sha256 for speed --- scripts/build_go_slim.sh | 18 ++++++---- site/site.go | 73 +++++++++++++++++++++++++++------------- site/site_test.go | 49 +++++++++++++-------------- 3 files changed, 84 insertions(+), 56 deletions(-) diff --git a/scripts/build_go_slim.sh b/scripts/build_go_slim.sh index e4328f6988fdd..21d70b9918b8c 100755 --- a/scripts/build_go_slim.sh +++ b/scripts/build_go_slim.sh @@ -111,17 +111,21 @@ done if [[ $compress != 0 ]]; then pushd "$dest_dir" - log "--- Generating SHA256 for coder-slim binaries ($dest_dir/coder.sha256)" - shasum -b -a 256 coder-* | tee coder.sha256 - echo "$dest_dir/coder.sha256" + sha_file=coder.sha1 + sha_dest="$dest_dir/$sha_file" + log "--- Generating SHA1 for coder-slim binaries ($sha_dest)" + shasum -b -a 1 coder-* | tee $sha_file + echo "$sha_dest" log log - log "--- Compressing coder-slim binaries using zstd level $compress ($dest_dir/coder.tar.zst)" - tar cf coder.tar coder.sha256 coder-* + tar_name=coder.tar.zst + tar_dest="$dest_dir/$tar_name" + log "--- Compressing coder-slim binaries using zstd level $compress ($tar_dest)" + tar cf coder.tar $sha_file coder-* rm coder-* - zstd --force --ultra --long -"${compress}" --rm --no-progress coder.tar -o coder.tar.zst - echo "$dest_dir/coder.tar.zst" + zstd --force --ultra --long -"${compress}" --rm --no-progress coder.tar -o $tar_name + echo "$tar_dest" log log diff --git a/site/site.go b/site/site.go index e7877d99aaccc..2c42418270381 100644 --- a/site/site.go +++ b/site/site.go @@ -4,11 +4,10 @@ import ( "archive/tar" "bytes" "context" - "crypto/sha256" + "crypto/sha1" "encoding/hex" "errors" "fmt" - "hash" "io" "io/fs" "net/http" @@ -23,6 +22,7 @@ import ( "github.com/klauspost/compress/zstd" "github.com/unrolled/secure" "golang.org/x/exp/slices" + "golang.org/x/sync/errgroup" "golang.org/x/xerrors" ) @@ -442,9 +442,9 @@ func ExtractOrReadBinFS(dest string, siteFS fs.FS) (http.FileSystem, error) { return nil, err } - ok, err := verifyBinSha265IsCurrent(dest, siteFS) + ok, err := verifyBinSha1IsCurrent(dest, siteFS) if err != nil { - return nil, xerrors.Errorf("verify coder binaries sha256 failed: %w", err) + return nil, xerrors.Errorf("verify coder binaries sha1 failed: %w", err) } if !ok { n, err := extractBin(dest, archive) @@ -470,62 +470,89 @@ func filterFiles(files []fs.DirEntry, names ...string) []fs.DirEntry { return filtered } -func verifyBinSha265IsCurrent(dest string, siteFS fs.FS) (ok bool, err error) { - b1, err := fs.ReadFile(siteFS, "bin/coder.sha256") +// errHashMismatch is a sentinel error used in verifyBinSha1IsCurrent. +var errHashMismatch = xerrors.New("hash mismatch") + +func verifyBinSha1IsCurrent(dest string, siteFS fs.FS) (ok bool, err error) { + b1, err := fs.ReadFile(siteFS, "bin/coder.sha1") if err != nil { - return false, xerrors.Errorf("read coder sha256 from embedded fs failed: %w", err) + return false, xerrors.Errorf("read coder sha1 from embedded fs failed: %w", err) } - // Parse sha256 file. + // Parse sha1 file. shaFiles := make(map[string][]byte) for _, line := range bytes.Split(bytes.TrimSpace(b1), []byte{'\n'}) { parts := bytes.Split(line, []byte{' ', '*'}) if len(parts) != 2 { - return false, xerrors.Errorf("malformed sha256 file: %w", err) + return false, xerrors.Errorf("malformed sha1 file: %w", err) } shaFiles[string(parts[1])] = parts[0] } if len(shaFiles) == 0 { - return false, xerrors.Errorf("empty sha256 file: %w", err) + return false, xerrors.Errorf("empty sha1 file: %w", err) } - b2, err := os.ReadFile(filepath.Join(dest, "coder.sha256")) + b2, err := os.ReadFile(filepath.Join(dest, "coder.sha1")) if err != nil { if xerrors.Is(err, fs.ErrNotExist) { return false, nil } - return false, xerrors.Errorf("read coder sha256 failed: %w", err) + return false, xerrors.Errorf("read coder sha1 failed: %w", err) } + var eg errgroup.Group + // Speed up startup by verifying files concurrently. Concurrency + // is limited to save resources / early-exit. Early-exit speed + // could be improved by using a context aware io.Reader and + // passing the context from errgroup.WithContext. + eg.SetLimit(3) + // Verify the hash of each on-disk binary. - sha := sha256.New() for file, hash1 := range shaFiles { - sha.Reset() - hash2, err := hashFile(sha, filepath.Join(dest, file)) - if err != nil { - return false, xerrors.Errorf("hash file failed: %w", err) - } - if !bytes.Equal(hash1, hash2) { + file := file + hash1 := hash1 + eg.Go(func() error { + hash2, err := sha1HashFile(filepath.Join(dest, file)) + if err != nil { + if xerrors.Is(err, fs.ErrNotExist) { + return errHashMismatch + } + return xerrors.Errorf("hash file failed: %w", err) + } + if !bytes.Equal(hash1, hash2) { + return errHashMismatch + } + return nil + }) + } + err = eg.Wait() + if err != nil { + if xerrors.Is(err, errHashMismatch) { return false, nil } + return false, err } return bytes.Equal(b1, b2), nil } -func hashFile(sum hash.Hash, name string) ([]byte, error) { +// sha1HashFile computes a SHA1 hash of the file, returning the hex +// representation. +func sha1HashFile(name string) ([]byte, error) { + //#nosec // Not used for cryptography. + hash := sha1.New() f, err := os.Open(name) if err != nil { return nil, err } defer f.Close() - _, err = io.Copy(sum, f) + _, err = io.Copy(hash, f) if err != nil { return nil, err } - b := make([]byte, sum.Size()) - sum.Sum(b[:0]) + b := make([]byte, hash.Size()) + hash.Sum(b[:0]) return []byte(hex.EncodeToString(b)), nil } diff --git a/site/site_test.go b/site/site_test.go index 510a211cd3e08..3c5ffb7068450 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -1,7 +1,6 @@ package site_test import ( - "bufio" "bytes" "context" "encoding/json" @@ -181,37 +180,35 @@ func TestShouldCacheFile(t *testing.T) { } const ( - binCoderSha256 = "bin/coder.sha256" + binCoderSha1 = "bin/coder.sha1" binCoderTarZstd = "bin/coder.tar.zst" ) func sampleBinFS() fstest.MapFS { return fstest.MapFS{ - binCoderSha256: &fstest.MapFile{ - Data: []byte("9da308c2e4bc33afa72df5c088b5fc5673c477f3ef21d6bdaa358393834f9804 *coder-linux-amd64\n"), + binCoderSha1: &fstest.MapFile{ + Data: []byte("55641d5d56bbb8ccf5850fe923bd971b86364604 *coder-linux-amd64\n"), }, binCoderTarZstd: &fstest.MapFile{ // echo -n compressed >coder-linux-amd64 - // shasum -b -a 256 coder-linux-amd64 | tee coder.sha256 - // tar cf coder.tar coder-linux-amd64 + // shasum -b -a 1 coder-linux-amd64 | tee coder.sha1 + // tar cf coder.tar coder.sha1 coder-linux-amd64 // zstd --long --ultra -22 coder.tar Data: []byte{ - 0x28, 0xb5, 0x2f, 0xfd, 0x64, 0x00, 0x27, 0x4d, 0x05, 0x00, 0xf2, 0x49, - 0x1e, 0x1a, 0x90, 0xb7, 0x0d, 0x00, 0x8d, 0x0d, 0x68, 0x68, 0xea, 0x6b, - 0x77, 0x17, 0xec, 0xce, 0xb2, 0x97, 0x24, 0x25, 0x45, 0x2e, 0x4d, 0xbf, - 0xeb, 0x46, 0x94, 0x01, 0xa7, 0x56, 0x6b, 0x4b, 0x39, 0x20, 0x49, 0x3a, - 0xab, 0xda, 0xae, 0x89, 0x07, 0x60, 0x57, 0xc8, 0x17, 0x89, 0xc5, 0x40, - 0x28, 0x14, 0x01, 0xc3, 0x88, 0x0c, 0xc6, 0x8b, 0xd6, 0x58, 0xd4, 0xfb, - 0xaf, 0x0e, 0x57, 0xa9, 0x6c, 0x2d, 0x3b, 0x87, 0xd3, 0x1e, 0xef, 0xc4, - 0xfa, 0x40, 0x08, 0x53, 0x33, 0xa5, 0xe7, 0xd9, 0xef, 0xa6, 0x83, 0x97, - 0x26, 0x84, 0xfb, 0xca, 0x38, 0xf6, 0xc8, 0x19, 0xa3, 0x3c, 0xa2, 0x2d, - 0x87, 0xe6, 0x1c, 0x35, 0x13, 0xcf, 0x8f, 0x2b, 0x2f, 0xb7, 0x72, 0x1e, - 0x6b, 0x62, 0x9f, 0x64, 0x76, 0x3c, 0x6f, 0xfd, 0x41, 0x13, 0x28, 0x4b, - 0x25, 0x29, 0x11, 0x20, 0x50, 0x2b, 0x2e, 0x6d, 0x03, 0xf2, 0x21, 0xef, - 0xc7, 0xa8, 0x04, 0xc0, 0xb5, 0x0d, 0x1c, 0x20, 0x53, 0x08, 0xee, 0x0c, - 0xa0, 0x62, 0xa5, 0x42, 0x11, 0xc1, 0xa0, 0x49, 0x36, 0x5a, 0x21, 0x93, - 0x04, 0x69, 0x38, 0x3b, 0x20, 0xb3, 0x03, 0x5c, 0x34, 0xf5, 0x2f, 0x1f, - 0xeb, 0x39, 0xe8, + 0x28, 0xb5, 0x2f, 0xfd, 0x64, 0x00, 0x27, 0xb5, 0x04, 0x00, 0x12, 0x08, + 0x1a, 0x1a, 0x90, 0xa7, 0x0e, 0x00, 0x0c, 0x19, 0x7c, 0xfb, 0xa0, 0xa1, + 0x5d, 0x21, 0xee, 0xae, 0xa8, 0x35, 0x65, 0x26, 0x57, 0x6e, 0x9a, 0xee, + 0xaf, 0x77, 0x94, 0x01, 0xf8, 0xec, 0x3d, 0x86, 0x1c, 0xdc, 0xb1, 0x76, + 0x8d, 0x31, 0x8a, 0x00, 0xf6, 0x77, 0xa9, 0x48, 0x24, 0x06, 0x42, 0xa1, + 0x08, 0x14, 0x4e, 0x67, 0x5f, 0x47, 0x4a, 0x8f, 0xf1, 0x6a, 0x8d, 0xc1, + 0x5a, 0x36, 0xea, 0xb6, 0x16, 0x52, 0x4a, 0x79, 0x7f, 0xbf, 0xb2, 0x77, + 0x63, 0x4b, 0x0e, 0x4b, 0x41, 0x12, 0xe2, 0x25, 0x98, 0x05, 0x73, 0x53, + 0x35, 0x71, 0xf5, 0x68, 0x37, 0xb7, 0x61, 0x45, 0x3e, 0xd9, 0x47, 0x99, + 0x3d, 0x51, 0xd3, 0xe0, 0x09, 0x10, 0xf6, 0xc7, 0x0a, 0x10, 0x20, 0x50, + 0x2b, 0x2e, 0x6d, 0x03, 0xf2, 0x21, 0xef, 0xc7, 0xa8, 0xc4, 0x3b, 0x8c, + 0x03, 0x64, 0x1a, 0xd9, 0x9d, 0x01, 0x60, 0xac, 0x94, 0x5a, 0x08, 0x05, + 0x4d, 0xb2, 0xd1, 0x0a, 0x99, 0x14, 0x48, 0xe3, 0xd9, 0x01, 0x99, 0x1d, + 0xe0, 0xda, 0xd4, 0xbd, 0xd4, 0xc6, 0x51, 0x0d, }, }, } @@ -240,7 +237,7 @@ func TestServingBin(t *testing.T) { copy(sampleBinFSCorrupted[binCoderTarZstd].Data[10:], bytes.Repeat([]byte{0}, 10)) // Zero portion of archive. sampleBinFSMissingSha256 := sampleBinFS() - delete(sampleBinFSMissingSha256, binCoderSha256) + delete(sampleBinFSMissingSha256, binCoderSha1) type req struct { url string @@ -274,7 +271,7 @@ func TestServingBin(t *testing.T) { { name: "Error on malformed sha256 file", fs: fstest.MapFS{ - binCoderSha256: &fstest.MapFile{Data: []byte("byebye")}, + binCoderSha1: &fstest.MapFile{Data: []byte("byebye")}, binCoderTarZstd: sampleBinFS()[binCoderTarZstd], }, wantErr: true, @@ -282,7 +279,7 @@ func TestServingBin(t *testing.T) { { name: "Error on empty archive", fs: fstest.MapFS{ - binCoderSha256: &fstest.MapFile{Data: []byte{}}, + binCoderSha1: &fstest.MapFile{Data: []byte{}}, binCoderTarZstd: &fstest.MapFile{Data: []byte{}}, }, wantErr: true, @@ -436,7 +433,7 @@ func TestExtractOrReadBinFS(t *testing.T) { defer f.Close() got := make([]byte, 5) // hello - _, err = bufio.NewReader(f).Read(got) + _, err = f.Read(got) require.NoError(t, err) assert.NotEqual(t, dontWant, got, "file should be overwritten on hash mismatch") From f65bd4c7f408c599383e275e5b1c8cf1cc01e9c6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 22 Jun 2022 12:48:10 +0300 Subject: [PATCH 4/5] fix: Add nosec to crypto/sha1 import --- site/site.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/site.go b/site/site.go index 2c42418270381..53560a706d2d6 100644 --- a/site/site.go +++ b/site/site.go @@ -4,7 +4,7 @@ import ( "archive/tar" "bytes" "context" - "crypto/sha1" + "crypto/sha1" //#nosec // Not used for cryptography. "encoding/hex" "errors" "fmt" From 4e8bba0b5757dcb916e25b85ac77f5679c555322 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 22 Jun 2022 14:15:03 +0300 Subject: [PATCH 5/5] fix: Check shasum equality first for early-exit --- site/site.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/site/site.go b/site/site.go index 53560a706d2d6..f838e2b61645e 100644 --- a/site/site.go +++ b/site/site.go @@ -499,6 +499,11 @@ func verifyBinSha1IsCurrent(dest string, siteFS fs.FS) (ok bool, err error) { return false, xerrors.Errorf("read coder sha1 failed: %w", err) } + // Check shasum files for equality for early-exit. + if !bytes.Equal(b1, b2) { + return false, nil + } + var eg errgroup.Group // Speed up startup by verifying files concurrently. Concurrency // is limited to save resources / early-exit. Early-exit speed @@ -532,7 +537,7 @@ func verifyBinSha1IsCurrent(dest string, siteFS fs.FS) (ok bool, err error) { return false, err } - return bytes.Equal(b1, b2), nil + return true, nil } // sha1HashFile computes a SHA1 hash of the file, returning the hex