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

scaffolded controller-gen dependency is not checksummed #1429

Closed
toonsevrin opened this issue Mar 12, 2020 · 3 comments
Closed

scaffolded controller-gen dependency is not checksummed #1429

toonsevrin opened this issue Mar 12, 2020 · 3 comments

Comments

@toonsevrin
Copy link
Contributor

toonsevrin commented Mar 12, 2020

I'm not sure what the security requirements for this project are but the scaffolded controller-gen dependency is not checksummed. This makes the project vulnerable to for example sigs.k8s.io being compromised or inconsistencies when someone forces a new commit on the {{ .ControllerToolsVersion }} tag.

pkg/scaffold/internal/templates/v2/makefile.go#L128

go get sigs.k8s.io/controller-tools/cmd/controller-gen@{{ .ControllerToolsVersion }} ;

A go.sum should be added to the Makefile template (near the line I quoted):

echo sigs.k8s.io/controller-tools/cmd/controller-gen {{ .ControllerToolsVersion }}  {{ .ControllerToolsChecksum }}  > go.sum

I'm not sure if a go.sum works without a go.mod file though, please confirm!

@camilamacedo86
Copy link
Member

camilamacedo86 commented May 7, 2020

HI @toonsevrin,

Go 1.13+ setup by default its proxies and the modules are download from there usually. IHMO add a checksum as you suggested is a little overwhelming. However, users are able to customize their Makefile as they wish and then. So, if they think that would be beneficial and required for the work and develop their project check the checksum as you suggested they are able to do it. WDYT? Would not it be enough?

@toonsevrin
Copy link
Contributor Author

Yep :) and checksummed against sum.golang.org or something (so an attacker would have to take over sum.golang.org and proxy.golang.org).

And I guess so, I believe checksumming dependencies should be the standard, especially in open-source projects. But as this is an ideological belief (see the note above about this being quite secure), I won't argue you further :)

@camilamacedo86
Copy link
Member

IHMO is better we try to keep the things as simple as possible but we need to allow users to customize it as they wish. Also, if someone has too much concern could indeed not allow doing the download and instead, use what is available in the machine already.

We have also the RFE:#1351

So, I think we could close this one. WDYT? Feel free to keep open if besides the above argumentation you think that it is something that should be addressed anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants