-
Notifications
You must be signed in to change notification settings - Fork 372
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
Build fails if CGO_ENABLED=0 #1634
Comments
I’m afraid it’s pretty much impossible to get a functionally equivalent alternative without CGo — because in a garbage-collected language, Go code has no control about copies of private key material, and no capability to erase it after use (which was a theoretical concern right until Heartbleed). (The lack of zeroization doesn’t technically prevent users from working with private keys in Go, and many do, of course.) Similarly side-channel-minimizing implementation are pretty hard to do in a higher-level language. So there are trade-offs. These trade-off manifest in that the Another trade-off is that any extra alternative implementations are not likely to be very well maintained. The No amount of automatic magic can make these trade-offs go away so that users just don’t need to care — especially if there really are platforms that plain can’t use CGo; in that case we can’t do a good job with any amount of effort. Note that the package does compile without CGo: Learn about the trade-off, make a choice, and add the build tag. Yes, that breaks plain |
@mtrmac thanks for the background information But this issue is still not solved; building on platforms without cgo fails. There seems to be two solutions: add stubs that return errors when no implementation is compiled in, and use those for !cgo, or auto enable openpgp if !cgo. Since the latter was already rejected, are we all good with the former? I'll send a pr... |
I think the former should be ok. Just functionality will be limited. |
IMHO the user should just make a choice and add a tag. That’s not impossible, is it? Hypothetically, I think you wouldn’t accept a Silently rejecting to even validate signatures could result in applications that compile OK and just refuse to pull images. If anyone doesn’t have robust testing, that can lead to pointless production outages. |
The build fails with an error that doesn't make it clear why the build is failing. At minimum I would expect the build to fail with something like "ImageSigningRequiresCgo is not defined" and not "fooBar() undefined" Also, this code is being used in non-cgo environments. Maybe not the original intent of the code, but it's the way it is. It should work without cgo, regardless of the security implications. A tag opt-in to avoid a compile error is fine, but the compile error should make it clear why the build is failing. |
I don’t think we agree on basic values enough to continue this discussion. |
Not sure how it's "basic values" to believe that the code should work without cgo. I was suggesting to stub the package to fix the build error to be clearer, as an alternative. |
@mtrmac from your readme...
How is this different? The other tag you've got also is stubbed. |
Everyone is busy all the time, and no-one knows everything about everything. so we as engineers are responsible for making code secure by default. The convenient and lazy thing to do must be the sufficiently secure one. Users have always the freedom to take the extra effort to make the less secure choice — ultimately, by forking, or using another product, sure — but to make the low-effort path the dangerous one is abdicating responsibility and trust placed in us. |
@mtrmac Understood, I'll send a PR which at least changes the build error from:
To...
... at least that way it's clear that it's intentional and meant to be left that way. |
Add the buildtag to your error message to tell the user, if they want to disable signing, you can use this flag. |
@rhatdan do you mean add a new build tag which has a stub that returns errors instead of failing the build? |
Yes if there is a buildtag that states, without_pgp or something like that then the error for no cgo should suggest it. |
@rhatdan there is not currently a tag which builds a stub returning an error - I could add one, maybe |
I believe that would satisfy @mtrmac desire to make users explicitly disable the security feature. |
@rhatdan The PR I sent: If CGO_ENABLED=0 the build fails with a warning saying Cgo is required. They would have to explicitly enable openpgp (considered insecure) to make the build pass. So I think that already satisfies what @mtrmac is saying which is that the "insecure" openpgp implementation should not be compiled unless a build flag is specified. The "default go build" output should always be considered secure. |
If you wanted to add a build tag - say //go:build containers_image_disable_signing
// +build containers_image_disable_signing
package signature
// ErrSigningDisabled is returned if container image signing is disabled.
var ErrSigningDisabled = errors.New("container image signing is disabled in this build")
// newGPGSigningMechanismInDirectory returns an error indicating signing is disabled.
func newGPGSigningMechanismInDirectory(optionalDir string) (SigningMechanism, error) {
return nil, ErrSigningDisabled
}
// newEphemeralGPGSigningMechanism returns an error indicating signing is disabled.
func newEphemeralGPGSigningMechanism(blobs [][]byte) (SigningMechanism, []string, error) {
return nil, nil, ErrSigningDisabled
} |
@rhatdan Added to #1638 - the first commit clarifies the build failure error message & adds a comment explaining why. The second commit adds a new tag which compiles cleanly with a stub returning a runtime error instead of a build failure. Hopefully that clears up all use cases:
|
…use CGO For discussion see containers/image#1634
I'm working on a Buildroot package for Podman.
Cgo is not available on many platforms. libgpgme (used for signing) requires cgo.
#1632 (closed) proposed to enable openpgp when cgo is unavailable.
The package should compile when
CGO_ENABLED=0
- with some kind of alternative implementation to libgpgme that is go-only.The text was updated successfully, but these errors were encountered: