From 3fe8bcc57c8b2b2f78b8860be1d12cccfc2ed56b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Mon, 19 Jul 2021 10:29:57 -0400 Subject: [PATCH] Replace getFileMD5 with getFileSHA256 The function was using an MD5 for checking, whereas a more secure option like SHA-256 would be preferred. See https://www.iacr.org/cryptodb/data/paper.php?pubkey=23903 (for example) for the (in-)security. SHA256 was chosen over Blake2 because there's a widely deployed CLI sha256sum on unix systems, and performance is not an issue here. Activates gosec G401 to fix such an issue, activates G501, G502, G503, G505 to tie up loose ends on less-secure hash functions.. --- .golangci.yml | 5 +++++ cmd/bootstrap/transit/cmd/crypto_test.go | 6 +++--- cmd/bootstrap/transit/cmd/pull.go | 8 ++++---- cmd/bootstrap/transit/cmd/utils.go | 7 +++---- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9b1d777b977..7c0cac2a059 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,7 +13,12 @@ linters-settings: # To select a subset of rules to run. # Available rules: https://github.com/securego/gosec#available-rules includes: + - G401 - G402 + - G501 + - G502 + - G503 + - G505 linters: enable: diff --git a/cmd/bootstrap/transit/cmd/crypto_test.go b/cmd/bootstrap/transit/cmd/crypto_test.go index c82d9f45442..c31e0138290 100644 --- a/cmd/bootstrap/transit/cmd/crypto_test.go +++ b/cmd/bootstrap/transit/cmd/crypto_test.go @@ -59,7 +59,7 @@ func TestEndToEnd(t *testing.T) { } } -func TestMd5(t *testing.T) { +func TestSha256(t *testing.T) { tmpFile, err := ioutil.TempFile(os.TempDir(), "prefix-") assert.NoError(t, err) defer os.Remove(tmpFile.Name()) @@ -70,7 +70,7 @@ func TestMd5(t *testing.T) { assert.NoError(t, tmpFile.Close()) - md5, err := getFileMD5(tmpFile.Name()) + hash, err := getFileSHA256(tmpFile.Name()) assert.NoError(t, err) - assert.Equal(t, "1b8e86521e7e04d647faa9e6192a65f5", md5) + assert.Equal(t, "876a3eab5fe740cb864a3d62869b0eefd6fbc34ec331c3064a6ffac0f9485a88", hash) } diff --git a/cmd/bootstrap/transit/cmd/pull.go b/cmd/bootstrap/transit/cmd/pull.go index 449bf1eef91..7085cbcd88c 100644 --- a/cmd/bootstrap/transit/cmd/pull.go +++ b/cmd/bootstrap/transit/cmd/pull.go @@ -114,11 +114,11 @@ func pull(cmd *cobra.Command, args []string) { } } - // calculate MD5 of rootsnapshot + // calculate SHA256 of rootsnapshot rootFile := filepath.Join(flagBootDir, model.PathRootProtocolStateSnapshot) - rootMD5, err := getFileMD5(rootFile) + rootSHA256, err := getFileSHA256(rootFile) if err != nil { - log.Fatal().Err(err).Str("file", rootFile).Msg("failed to calculate md5") + log.Fatal().Err(err).Str("file", rootFile).Msg("failed to calculate SHA256 of root file") } - log.Info().Str("md5", rootMD5).Msg("calculated MD5 of protocol snapshot") + log.Info().Str("sha256", rootSHA256).Msg("calculated SHA256 of protocol snapshot") } diff --git a/cmd/bootstrap/transit/cmd/utils.go b/cmd/bootstrap/transit/cmd/utils.go index 7e3b63bb430..827b7d5c147 100644 --- a/cmd/bootstrap/transit/cmd/utils.go +++ b/cmd/bootstrap/transit/cmd/utils.go @@ -1,8 +1,8 @@ package cmd import ( - "crypto/md5" "crypto/rand" + "crypto/sha256" "fmt" "io" "io/ioutil" @@ -73,18 +73,17 @@ func getFilesToUpload(role flow.Role) []string { } } -func getFileMD5(file string) (string, error) { +func getFileSHA256(file string) (string, error) { f, err := os.Open(file) if err != nil { return "", err } defer f.Close() - h := md5.New() + h := sha256.New() if _, err := io.Copy(h, f); err != nil { return "", err } - return fmt.Sprintf("%x", h.Sum(nil)), nil }