Add support for validating GitHub Release Asset checksums #34
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "crypto/sha256" | ||
| "crypto/sha512" | ||
| "os" | ||
| "io" | ||
| "hash" | ||
| "encoding/hex" | ||
| ) | ||
|
|
||
| func verifyChecksumOfReleaseAsset(assetPath, checksum, algorithm string) *FetchError { | ||
| computedChecksum, err := computeChecksum(assetPath, algorithm) | ||
| if err != nil { | ||
| return newError(ERROR_WHILE_COMPUTING_CHECKSUM, err.Error()) | ||
|
There was a problem hiding this comment. It's generally nicer when writing tests and debugging to return a different type for each, well, type of error. All you do is define a struct (or even a type alias) and add an There was a problem hiding this comment. I hear you. This was an early Golang tool I wrote and I didn't want to refactor all the errors throughout the code so I stuck with the existing idiom. |
||
| } | ||
| if computedChecksum != checksum { | ||
| return newError(CHECKSUM_DOES_NOT_MATCH, fmt.Sprintf("Expected to receive checksum value %s, but instead got %s for Release Asset at %s. This means that either you are using the wrong checksum value in your call to fetch (e.g., did you update the version of the module you're installing but not the checksum?) or that someone has replaced the asset with a potentially dangerous one and you should be very careful about proceeding.", computedChecksum, checksum, assetPath)) | ||
| } | ||
|
|
||
| fmt.Printf("Checksum matches!") | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func computeChecksum(filePath string, algorithm string) (string, error) { | ||
| fmt.Printf("Computing checksum of release asset\n") | ||
|
|
||
| file, err := os.Open(filePath) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer file.Close() | ||
|
|
||
| hasher, err := getHasher(algorithm) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| _, err = io.Copy(hasher, file) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return hasherToString(hasher), nil | ||
| } | ||
|
|
||
| // Return a hasher instance, the common interface used by all Golang hashing functions | ||
| func getHasher(algorithm string) (hash.Hash, error) { | ||
| switch algorithm { | ||
| case "sha256": | ||
| return sha256.New(), nil | ||
| case "sha512": | ||
| return sha512.New(), nil | ||
| default: | ||
| return nil, fmt.Errorf("The checksum algorithm \"%s\" is not supported", algorithm) | ||
| } | ||
| } | ||
|
|
||
| // Convert a hasher instance to the string value of that hasher | ||
| func hasherToString(hasher hash.Hash) string { | ||
| return hex.EncodeToString(hasher.Sum(nil)) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "testing" | ||
| "github.com/stretchr/testify/assert" | ||
| "io/ioutil" | ||
| ) | ||
|
|
||
| const SAMPLE_RELEASE_ASSET_GITHUB_REPO_URL ="https://github.com/gruntwork-io/health-checker" | ||
| const SAMPLE_RELEASE_ASSET_VERSION="v0.0.2" | ||
| const SAMPLE_RELEASE_ASSET_NAME="health-checker_linux_amd64" | ||
|
|
||
| // Checksums can be computed by running "shasum -a [256|512] /path/to/file" on any UNIX system | ||
| const SAMPLE_RELEASE_ASSET_CHECKSUM_SHA256="4314590d802760c29a532e2ef22689d4656d184b3daa63f96bc8b8f76f5d22f0" | ||
| const SAMPLE_RELEASE_ASSET_CHECKSUM_SHA512="28d9e487c1001e3c28d915c9edd3ed37632f10b923bd94d4d9ac6d28c0af659abbe2456da167763d51def2182fef01c3f73c67edf527d4ed1389a28ba10db332" | ||
|
There was a problem hiding this comment. Where do you users get these checksums from? If it's from the There was a problem hiding this comment. Exactly. When I went to download a third-party health checker for Kafka, I realized how risky (and yet common!) including such a library is. If an attacker wanted to exploit a library that many people use, one way is to find a way into someone's GitHub account (leaked GitHub token?), leave the commits unaltered and update only the release asset (where in most cases no checksum at all is published). In this case, I'm making the assumption that at the time you download the code, you have an opportunity to look around the repo and conclude for yourself that it appears not to be compromised. There's only so much you can do here, of course, but at least if in the future when you've long forgotten about this library, something malicious happens, you'll be notified. In the end, I suppose it's just more defense in depth. |
||
|
|
||
| func TestVerifyReleaseAsset(t *testing.T) { | ||
| tmpDir := mkTempDir(t) | ||
|
|
||
| githubRepo, err := ParseUrlIntoGitHubRepo(SAMPLE_RELEASE_ASSET_GITHUB_REPO_URL, "") | ||
| if err != nil { | ||
| t.Fatalf("Failed to parse sample release asset GitHub URL into Fetch GitHubRepo struct: %s", err) | ||
| } | ||
|
|
||
| assetPath, fetchErr := downloadReleaseAsset(SAMPLE_RELEASE_ASSET_NAME, tmpDir, githubRepo, SAMPLE_RELEASE_ASSET_VERSION) | ||
| if fetchErr != nil { | ||
| t.Fatalf("Failed to download release asset: %s", fetchErr) | ||
| } | ||
|
|
||
| checksumSha256, fetchErr := computeChecksum(assetPath, "sha256") | ||
| if fetchErr != nil { | ||
| t.Fatalf("Failed to compute file checksum: %s", fetchErr) | ||
| } | ||
|
|
||
| checksumSha512, fetchErr := computeChecksum(assetPath, "sha512") | ||
| if fetchErr != nil { | ||
| t.Fatalf("Failed to compute file checksum: %s", fetchErr) | ||
| } | ||
|
|
||
| assert.Equal(t, SAMPLE_RELEASE_ASSET_CHECKSUM_SHA256, checksumSha256, "SHA256 checksum of sample asset failed to match.") | ||
| assert.Equal(t, SAMPLE_RELEASE_ASSET_CHECKSUM_SHA512, checksumSha512, "SHA512 checksum of sample asset failed to match.") | ||
| } | ||
|
|
||
| func mkTempDir(t *testing.T) string { | ||
| tmpDir, err := ioutil.TempDir("", "") | ||
| if err != nil { | ||
| t.Fatalf("Failed to create temp directory: %s", err) | ||
| } | ||
|
|
||
| return tmpDir | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems equally important to have checksums for modules from the
--modulesparam. Probably the simplest option is to publish the commit ID (sha1) and compare that to the commit ID of the repo we just downloaded viagit clone.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean the
--modulespattern ofgruntwork-install? Well, can't you just use Fetch to download the exact commit you want to achieve what you're suggesting?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
--modules. We typically use tags for specifying the version we want. Should our recommendation be that users specify both the tag and the commit ID?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, keep in mind that fetch allows you to use tag constrain expressions, but I see where you're going with this. Yes, I think we should permit using both a
--tagand a--commit-idparam at the same time. Fetch can then error out if the two don't match. I think I'll file a GitHub issue for that for now, though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#35