Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/vgo: rename go.modverify to go.sum and enable by default #25525

Closed
FiloSottile opened this issue May 23, 2018 · 10 comments
Closed

x/vgo: rename go.modverify to go.sum and enable by default #25525

FiloSottile opened this issue May 23, 2018 · 10 comments
Labels
FrozenDueToAge
Milestone
vgo

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented May 23, 2018 •
edited

go.modverify guarantees that only the main source repository needs to be securely obtained and verified for the whole build to be verified and reproducible. It also enables untrusted proxies.

Go has a culture of secure by default, so we should provide this safety automatically. Also, most people will be upgrading from dep, so we should not downgrade their security in the process.

A flag to disable modverify is probably not needed, if someone wants to turn off the brakes they can easily add the file to gitignore and/or script its removal.

@rsc suggested renaming it to go.sum, which sounds good to me.

This is not to say that we don't aim to build a better verification system that will hopefully replace go.sum files for most users, but that will take more time than vgo adoption will hopefully take.
@gopherbot gopherbot added this to the vgo milestone May 23, 2018
@gopherbot
Copy link

Change https://golang.org/cl/115496 mentions this issue: cmd/go/internal/vgo: rename go.modverify to go.sum

@rsc rsc self-assigned this Jun 6, 2018
@gopherbot
Copy link

Change https://golang.org/cl/121298 mentions this issue: cmd/go/internal/modfetch: vgo.fetch becomes modfetch.Download

@gopherbot
Copy link

Change https://golang.org/cl/121302 mentions this issue: cmd/go/internal/modfetch: always create and update go.sum

gopherbot pushed a commit to golang/vgo that referenced this issue Jun 29, 2018
@rsc
cmd/go/internal/modfetch: vgo.fetch becomes modfetch.Download
06e6643 
Refactoring so that all the cache directory management is in package modfetch.

While here, rename go.modverify to go.sum,
in preparation for forcing go.sum checking on
all the time.

Vgo will automatically move content from
go.modverify into go.sum when it sees it.

For golang/go#25525.

Change-Id: Id03286d52ca4923b35ac14b639e5885571397bba
Reviewed-on: https://go-review.googlesource.com/121298
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@dlsniper
Copy link
Contributor

A bit late to the ticket, but why not add a third column to the require() section with the hash? Or simply have a section at the end of the go.mod file named hashes() or sums() which has the contents of the go.sum file? That way the go.mod file will be self-contained and there won't be any issues about opt-in/opt-out or committing the file (or indeed polluting the repository with an additional file).

@CJ-Jackson
Copy link

CJ-Jackson commented Jun 30, 2018 •
edited

@dlsniper Bad idea, than we won't be able to add go.mod to .gitignore, the checksum is different between for example a windows and a linux build, I would say it's better to keep it as it is, that way we
can add go.sum to .gitignore without causing too much issues.

This is what happens if I try to use a windows checksum on a linux system.

vgo: downloading github.com/CJ-Jackson/siteCore v0.0.0-20180630082004-bcd274312f9d
vgo: verifying github.com/CJ-Jackson/siteCore@v0.0.0-20180630082004-bcd274312f9d: checksum mismatch
        downloaded: h1:5pBLEVZSweqZE3X1EwumfZLOTijYjbkR1X01tZDam/o=
        go.sum:     h1:TnGHBnjMxFlr6JIAAd6jnZrebVO6ONP4ec2vyze3Tco=

The way I got round that issues is to add go.sum to .gitignore. Git can only ignore a file, not a section of a file.

@dlsniper
Copy link
Contributor

@CJ-Jackson unfortunately I don't understand how this happens and because the example you provided is not publicly accessible I can't reproduce it. My suggestion is to run vgo mod -sync and then see if the issue still happens.

However it's concerning that you have two different checksums as it means that the build is not reliable, which is the whole point of having this checksum feature.

@CJ-Jackson
Copy link

@dlsniper I had a more closer look it's zip the source code and generate the sum based on the zip file, hench .ziphash, I've run shasum on both the zip file and the mod in cache.

On windows (cygwin) I get this.

$ shasum v0.0.0-20180630082004-bcd274312f9d.zip
54ae762d6f9bc4fb8303851d2375902acc9b5bf2 *v0.0.0-20180630082004-bcd274312f9d.zip

$ shasum v0.0.0-20180630082004-bcd274312f9d.mod
eb96238006be384383579a58a59dea63ae4ccd20 *v0.0.0-20180630082004-bcd274312f9d.mod

On linux I got this.

$ shasum v0.0.0-20180630082004-bcd274312f9d.zip
10656dffb311eb25ab7cd2714e09f176738f18ae  v0.0.0-20180630082004-bcd274312f9d.zip

$ shasum v0.0.0-20180630082004-bcd274312f9d.mod
eb96238006be384383579a58a59dea63ae4ccd20  v0.0.0-20180630082004-bcd274312f9d.mod

As you can see the checksum of mod files are the same on windows and linux, but as for the zip file that comes out differently, that suggest to me that there might be a bug with the zip utility in Go. Until that is fix I have to leave go.sum on .gitignore.

@myitcv
Copy link
Member

myitcv commented Jun 30, 2018

@CJ-Jackson are you definitely running the same commit of vgo on each platform?

@CJ-Jackson
Copy link

CJ-Jackson commented Jun 30, 2018 •
edited

@myitcv Yes I am

Windows

$ git rev-parse --short HEAD
bbcfaa0

Linux

$ git rev-parse --short HEAD
bbcfaa0

I just notice a new commit in vgo, I'm going to update vgo and do the shasum again.

Update

I updated vgo to 0e237a4 on both linux and windows and I'm still getting different checksums for the zip file.

@sneko sneko mentioned this issue Jul 3, 2018
Closed
@rsc
Copy link
Contributor

rsc commented Jul 6, 2018

@CJ-Jackson's problem was git mangling the line endings, tracked as #26229.

@dlsniper In general we're much less likely to see comments on closed issues than on open ones. I just happened to come across this when working on #26229. To answer your question from 8 days ago, I can think of a few reasons to keep go.mod and go.sum separate:

  1. People who don't want a go.sum can .gitignore it.
  2. go.mod usually lists only direct requirements; go.sum must list all indirect requirements too.
  3. More than one version of a module can influence the final build list. We need somewhere to record those.
  4. To allow separate downloading of go.mod and the entire file tree for a given module version (because for many dependencies the file tree is not needed except for specific builds), we need to record two different hashes for each version.

These are all circling around the general concern, which is that go.mod is meant to be human-readable, with nothing more than a text editor, with meaningful diffs for code reviews, and so on. In contrast go.sum is very much not human-readable. It's an impenetrable alphabet soup. In the little repo I run tests in, at this moment go.mod is 803 bytes and go.sum is 7,470 bytes. If we put go.sum into go.mod the signal-to-noise ratio there would be near zero.

@doug-perlin doug-perlin mentioned this issue Jul 18, 2018
Merged
@golang golang locked and limited conversation to collaborators Jul 6, 2019
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
vgo
Development

No branches or pull requests
6 participants
@rsc @dlsniper @CJ-Jackson @FiloSottile @myitcv @gopherbot