fix: Hash concurrently and use sha1 vs sha256 for speed

coder · mafredri · Jun 22, 2022 · Jun 21, 2022 · Jun 21, 2022 · Jun 22, 2022
commit 5609369e521c1e2354e57d7f282e69f6857cd244
diff --git 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
@@ -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
@@ -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,15 +271,15 @@ 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,
 		},
 		{
 			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")